[devswha]

Summary

One admission rule for every way into a PC through the relay: a pairing code, a comparison of digits (#2696), and, with the follow-up PR, joining through an account. The rule is in docs/account.md ("Who may admit a device"), from #2834.

Stacked on #2834 (contract) and #2696.

• Who may approve. This PC itself (its driving windows, as in feat(relay): pair a phone without a code by comparing six digits #2696), or a drive account member connected through the relay. The member never sees the digits: GET /api/relay/pairings and the new remote-pairings event give it three candidates (choices), and POST /api/relay/pairing must name the new device's digits to approve. A wrong pick answers 409 wrong_digits and ends that comparison. Watch devices get read_only; other relay devices get account_required.
• Starting a code follows the same rule. Through the relay, POST /api/devices/pair/start is for drive account members only (403 account_required), since a code admits whoever holds it. On the PC itself nothing changes.
• Every admission is announced. device-paired carries approved_by (device id and label) when an account member approved it, and the new row keeps added_by. An approver revoked or demoted before the phone confirms admits nothing.
• Intent. A phone offers pair-intent-pc or pair-intent-account with pair-sas. The PC selects only the one offered, only alongside pair-sas, and a HELLO offering both ends with protocol_error. The prompt shows the intent. Older phones offer none (pc).
• Failures counted on two paths. Comparisons asking to join the account count their 20 failures in 10 minutes apart from codes and comparisons for this PC, so a flood of account requests cannot pause QR pairing. The cap of three waiting comparisons is shared.
• Headless approval. bun scripts/plugin.ts pair --approve lists the waiting devices with their digits and intent and asks y/n for each.
• docs/relay.md is updated for the above. src/ only skips the new event until the account screens land (acct-client lane).

Merge order note: until a device can become an account member (the follow-up PR, which applies link records), no relay device can start a code or approve remotely. Approval on the PC itself and pairing by QR code work as before. Merge this together with the PC-side PR, or after it.

Validation

• New contract tests in server/relay/pair-sas.contract.test.ts, over real RelaySessions with Noise, cover: intent negotiation; remote approval admits and records added_by; the announcement (with approved_by) reaches every connected device, and the candidates reach only the member; watch and non-member relay devices are refused while the request keeps waiting; a wrong pick refuses and counts on the account path only; account failures leave code pairing open; only members start codes through the relay; a revoked approver admits nothing; the cap of three covers account comparisons too.
• Each of these mutations turns a test red: no digits check in decideRemote, any relay device as approver, one shared failure counter, no approver re-check at admission, no member check for codes, the candidate stream to any relay device, and both intents accepted.
• Manual run of pair --approve against an isolated server (temp HOME and state dirs, port 0) with a real comparison waiting: it printed New phone wants to join your account. Approve it only if its screen shows 141 675. Approve? [y/N], answered y, and the phone got AUTH_OK.
• bun run check fast at ed1f7cc: workflow syntax, generated files, typecheck, build and unit tests (10972 pass, 0 fail) all ok.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

• Source mode: Auto
• Repository: devswha/ovis
• Branch: feat/account-admission
• File: .coderabbit.yaml
• Commit: fca71c1
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 1a213a44-f4f0-44b6-9ae4-2e601e141589

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• Trigger review

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[devswha]

Review in place of CodeRabbit

CodeRabbit skipped this PR because its base is not the default branch, so two review agents stand in (AGENTS.md: a PR touching authentication).

• omo code review at ed1f7cc: CLEAR.
• Codex review of the authorization rules at ed1f7cc. Findings and their resolutions, in d67b23a:

Severity	Finding	Reproduced	Resolution
HIGH	An approval survived its approver being demoted and promoted again before the phone confirmed: only the approver's role at admission was checked.	Yes (test)	The comparison now listens for the approver's revocation or role change from the moment it approves, and ends with auth_failed for good. New test: "an approval ends for good when its approver is given another role before the phone confirms"; it fails without the fix.
HIGH	pair-intent-account admits a plain row without account, and the docs said the approval carries the signed add.	Contract gap, not an authorization hole	The docs now match the design: either intent admits a plain row, and with pair-intent-account the new device links that row to the account itself (a link whose old_key is its key, in the PC-side PR). The approver delivers the add to the other PCs. Separate counts mean a relay in the middle gets about 40 abandoned comparisons in 10 minutes instead of 20 (one in 25,000). docs/relay.md now says so.
HIGH	An open LAN window can start a pairing code.	Yes, but it predates this PR	On an ungated install an open LAN window already has full access, so a code adds nothing to it. That is not changed here; the docs say "directly on the PC (any window there with drive access, as before)".
MEDIUM	A watch device on the PC's own address could read the digits of a waiting comparison from GET /api/relay/pairings and hand them to a remote approver.	Yes (test)	Watch devices are refused (read_only) in the dispatch watch guard. New test fails without it.
MEDIUM	The browser UI drops remote-pairings and does not show the intent in the PC's prompt.	n/a	By design for this PR: the screens are the client lane's (src/). Noted for it.
LOW	device-paired notices do not show approved_by yet.	n/a	Same: client lane.
LOW	A watch device starting a code through the relay gets read_only, not the documented account_required.	Yes	The docs now name both.

Number matching itself held: the candidates are three distinct values, shuffled with crypto.randomInt, and a wrong pick ends the request (a retry answers relay_pairing_gone). The two failure counters were found independent.

Validation at d67b23a: bun run check fast all ok (typecheck, build, unit tests 10972 pass, 0 fail). bun test server/relay/pair-sas.contract.test.ts: 27 pass.

[devswha]

Second review round

The omo code review of d67b23a answered BLOCK on one reproduced regression: through the relay, a driving phone could no longer start a pairing code (403 account_required), and until devices can become account members no phone could. Resolved in 68bf8cc: the account-only rule now applies once the PC belongs to an account. A PC in no account keeps letting any driving relay device start a code, as before. New test: "a PC in no account still lets a driving relay device start a pairing code" (it fails without the fix). The earlier rate-limit test is back to its original form.

Its LOW findings stay as they are:

• remote-pairings dropped by the browser, the intent missing from the PC's prompt, approved_by missing from the notice: the client lane's screens.
• pair --approve building its address from HOST: the same as the existing pair command, so not new here.

Validation at 68bf8cc: bun run check fast all ok (typecheck, build, unit tests 10972 pass, 0 fail). bun test server/relay/: 144 pass.

[cloudflare-workers-and-pages]

Deploying ovis with    Cloudflare Pages

Latest commit:	fca71c1
Status:	✅  Deploy successful!
Preview URL:	https://259eac38.ovis.pages.dev
Branch Preview URL:	https://feat-account-admission.ovis.pages.dev

View logs

[devswha]

Merged with main, and the client lane's additions

main (with #2696) and the updated contract are merged in, with no force-push. Where #2696 changed on main (the owner's pairing window, required features, last-seen on pairing), its version is kept, with this PR's rule layered on top. The account fixture opens the pairing window as main's fixture does.

Agreed with the client lane:

• remote-pairings entries carry key, the new device's P-256 key from PAIR_REQUEST, bound by its commitment and the digits.
• An approval answers { "ok": true, "device_id" }. Every row a comparison makes now takes that id, accountDeviceId(server_id, key).

The rule for joining through the account outside the owner's pairing window (design 5.3) lands in #2843.

The omo code review is CLEAR at fff5481.

Validation at fff5481: bun run check fast all ok (typecheck, build, unit tests 11398 pass, 0 fail). bun test server/relay/: all pass.

[devswha]

Landed as part of #2867 (the whole account stack was merged there in one squash).