Skip to content

fix(remote): stop a racing server write from restoring a revoked device - #86

Draft
Hao0321 wants to merge 1 commit into
mainfrom
security/remote-revoke-race
Draft

Hao0321 wants to merge 1 commit into
mainfrom
security/remote-revoke-race

Conversation

@Hao0321

@Hao0321 Hao0321 commented Oct 6, 2026

Copy link
Copy Markdown
Owner

What changed

The Remote server (src/remote/server.ts) rewrote the whole trusted-device file from a copy it had read earlier. It did this on pairing (LAN and relay), on lastSeen updates and on idle-expiry cleanup. The desktop's revoke_mobile_device rewrites the same file. If a server write landed just after a revoke, the device was written back and its credential worked again. #48 added more of these write paths.

  • Server: every trusted-device write goes through one queued helper, updateTrustedDevices. It re-reads the file immediately before writing and applies only its own change: add the newly paired device, update lastSeen of an entry that is still on disk, or drop an expired entry. It never writes back a device that is no longer on disk.
  • Desktop revocation list: two processes can still overlap between the server's fresh read and its write. To close that gap, revoke_mobile_device first records the revoked credential's hash in revoked-devices.json, next to the trusted file, and only then rewrites the trusted file.
    • It uses the existing owner-only atomic write_remote_json_atomic and holds the mobile_remote lock, so two revokes cannot lose each other's entry.
    • The list keeps the 256 most recent hashes.
    • The server never writes the list. It checks it on every lookup and drops listed devices from the trusted file on its next write.
    • No new environment variable: both sides derive the list's path from the trusted-device path.
  • Fail-closed: if the list exists but cannot be read, the server refuses every device and writes nothing. The desktop's next revoke replaces an unreadable list.
  • docs/SECURITY_MODEL.md: the Revocation bullet now describes both mechanisms.
  • .github/workflows/source-ci.yml: the Linux job runs only named Rust tests, so the new Rust test is added to its existing remote-state step. Job names are unchanged.

I rejected two alternatives:

  • A shared lock file would need stale-lock recovery, because the desktop kills the server process. Recovering by age brings the race back.
  • Restarting Remote on every revoke would disconnect all devices.

User journey and platform

Mobile Remote on the desktop. After you revoke a device in the Remote dialog, its next request is refused, even if it was active at the same moment. Other devices are unaffected.

Validation

  • New src/remote/revocation.test.ts: the server's read is held open on a FIFO so that a desktop-style revoke lands between the server's read and its write.
    • Paths covered: another device's activity, the revoked device's own activity, pairing and idle expiry.
    • Further tests: a fresh-read test without the list, a next-request-rejected test, a reappearing-entry test and an unreadable-list (fail-closed) test.
    • Before the fix: against server.ts on main, 7 of 8 tests fail (the revoked device gets 200).
    • After the fix: 8 pass, stable over 19 runs.
    • Platforms: the FIFO race tests are skipped on Windows, which has no mkfifo.
  • npx vitest run src/remote/: 76 passed, 1 skipped (that skip also exists on main).
  • npm test: 280 files and 2176 tests passed.
  • npm run typecheck, npm run build: passed.
  • npm run source:scan and npm run source:verify:self-test: GREEN.
  • scripts/architecture-check.ts: ALLOW.
  • Rust: with Rust 1.98.1 and the CI packages, cargo test --locked --manifest-path src-tauri/Cargo.toml --features community-desktop,tauri/custom-protocol revoking_a_mobile_device_lists_its_credential_before_dropping_it passed.
  • End-to-end check (a one-off, not committed): the real Rust revoke against the real Node server gave 200 before the revoke and 401 after. It stayed 401 when the entry was put back, and another device stayed 200.
  • Not run: Windows or macOS, the Tauri UI revoke flow and the native GTK smoke test.

Remaining limits:

  • A request whose reads finished just before the revoke is still served.
  • An unreadable revocation list blocks Remote until the next revoke, or until the file is fixed or deleted.

Security and provenance

No new dependencies, network access or process execution.

New file write: revoked-devices.json in the desktop's existing private remote-state directory. It is written only by the desktop, owner-only and atomic, and holds SHA-256 credential hashes only, never credentials.

No secrets or private footage.

DCO

The commit has a Signed-off-by: trailer.

🤖 Generated with Claude Code

https://claude.ai/code/session_01YQbUWKu73ka7dZ58vWVzgY


Generated by Claude Code

The Remote server rewrote the whole trusted-device file from a copy it had
read earlier (pairing, last-seen updates, idle expiry), while the desktop
revokes a device by rewriting the same file. A server write that landed
after a revoke put the device back, and its cookie or relay credential
worked again.

The server now applies each change, one at a time, to a fresh read of the
file and never writes back a device that is no longer on disk. A read and
a write in two processes can still straddle a revoke, so the desktop also
lists the revoked credential hash in revoked-devices.json, which only the
desktop writes. The server refuses every listed credential, drops it from
the trusted file on its next write, and fails closed when the list cannot
be read.

Signed-off-by: Hao0321 <126182090+Hao0321@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YQbUWKu73ka7dZ58vWVzgY
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant