Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions docs/account.md
Original file line number Diff line number Diff line change
Expand Up @@ -267,10 +267,14 @@ is what authenticates the PC.
close at once (`auth_revoked`). A `remove` signed before this PC itself paired the key again leaves
that pairing be (`unchanged`), so a late or replayed one cannot undo it. A row an `add` or a remote
approval made goes, however late the `remove` arrives.
- **Revoke** and **Revoke all** on the PC leave a tombstone too, so no record brings the key back.
- **Revoke** and **Revoke all** on the PC leave a tombstone too, so no record brings the key back,
even after the PC joins an account. An account `remove` for a key the PC revoked first takes over
its tombstone.
- A pairing made on this PC (a code started on the PC, or a comparison approved here) clears that
key's tombstone, after its row is written. A code started through the relay, or a comparison an
account member approves remotely, is refused for a tombstoned key.
account member approves remotely, is refused for a tombstoned key, except on a PC in no account:
there a code started through the relay may let back a key the PC revoked itself (not one the
account removed), and clears its tombstone.
- A stop between two such writes leaves a row and a tombstone for one key. On start the PC tells them
apart by time: a row made before the key's removal is revoked, one made after it keeps its device
and the tombstone goes. Until then no such row signs in, signs or approves.
Expand Down
105 changes: 105 additions & 0 deletions server/account/account.contract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { exportPublicKey, generateKeyPair, importPublicKey } from "../../shared/
import { bytesToHex } from "@noble/hashes/utils.js";
import { deriveServerId } from "../../shared/relay-server-proof.ts";
import { applyAccountRecords } from "./records.ts";
import { createServer } from "../index.ts";

type Fixture = ReturnType<typeof relayFixture>;
type Link = Awaited<ReturnType<typeof relayConnection>>;
Expand Down Expand Up @@ -751,6 +752,110 @@ test("a waiting remove survives a stop right after the service took its page's a
expect(f.app.context.services.devices.hasKey(c.spki)).toBe(false);
});

test("a key paired again in one of two state dirs stays paired, without its tombstone, when the other starts", async () => {
const f = fixture();
const { key, link } = await member(f);
const gone = await accountKey();
await post(link, "/api/account/records", { records: [await add(key, gone.spki)] });
await post(link, "/api/account/records", { records: [await remove(key, gone.spki)] });
const sibling = mkdtempSync(join(tmpdir(), "ovis-account-sibling-"));
// both dirs share the registry and the removal
for (const name of ["devices.json", "account-tombstones.json"]) writeFileSync(join(sibling, name), readFileSync(join(f.stateDir, name)), { mode: 0o600 });
const store = f.app.context.services.devices;
// a second after the removal, as a person pairing again would be
const later = Date.now() + 1_000;
expect(store.pairRelay(store.startPairing(later).code, "Mac app", gone.spki, later)).not.toBeNull();
const other = createServer({ port: 0, hostname: "127.0.0.1", stateDir: sibling, siblingStateDir: f.stateDir, token: "t", tailscaleOwner: null, machines: false, sidecar: false });
try {
// the merge brings the other dir's tombstone back, and the start-up repair drops it: the row is newer
for (const devices of [other.context.services.devices, new DeviceStore(f.stateDir)]) {
expect(devices.hasKey(gone.spki)).toBe(true);
expect(devices.isTombstoned(gone.spki)).toBe(false);
}
} finally {
other.stop();
rmSync(sibling, { recursive: true, force: true });
}
});

test("a key revoked before the PC joined an account stays out: a member's signed add for it is refused", async () => {
const f = fixture();
const store = f.app.context.services.devices;
const revoked = await accountKey();
const before = store.pairRelay(store.startPairing().code, "Old phone", revoked.spki);
if (!before) throw new Error("Expected relay enrollment");
store.revoke(before.device.id);
const { key, link } = await member(f);
const outcome = await post(link, "/api/account/records", { records: [await add(key, revoked.spki)] });
expect((outcome.body as unknown as { results: { outcome: string; code?: string }[] }).results[0]).toMatchObject({ outcome: "refused", code: "tombstoned" });
expect(store.hasKey(revoked.spki)).toBe(false);
});

test("an account remove for a key this PC revoked first makes its tombstone the account's", async () => {
const f = fixture();
const { key, link } = await member(f);
const store = f.app.context.services.devices;
const gone = await accountKey();
await post(link, "/api/account/records", { records: [await add(key, gone.spki)] });
store.revoke(accountDeviceId(SERVER_ID, gone.spki));
const removal = await remove(key, gone.spki);
await post(link, "/api/account/records", { records: [removal] });
const kept = JSON.parse(readFileSync(join(f.stateDir, "account-tombstones.json"), "utf8")) as { tombstones: { key: string; record_id: string }[] };
expect(kept.tombstones.find((t) => t.key === gone.spki)?.record_id).toBe(removal.id);
});

test("a key revoked on this PC stays out when a sibling dir that missed the revoke added it through the account", async () => {
const f = fixture();
const { key, link } = await member(f);
const store = f.app.context.services.devices;
const gone = await accountKey();
await post(link, "/api/account/records", { records: [await add(key, gone.spki)] });
const sibling = mkdtempSync(join(tmpdir(), "ovis-account-sibling-"));
try {
// the sibling dir still has the account's row; this dir then revokes it on the PC
writeFileSync(join(sibling, "devices.json"), readFileSync(join(f.stateDir, "devices.json")), { mode: 0o600 });
store.revoke(accountDeviceId(SERVER_ID, gone.spki));
const registry = JSON.parse(readFileSync(join(sibling, "devices.json"), "utf8")) as { devices: { relay_public_key?: string; created_at: string }[] };
// made a second after the revoke, as the sibling knew nothing of it
for (const row of registry.devices) if (row.relay_public_key === gone.spki) row.created_at = new Date(Date.now() + 1_000).toISOString();
writeFileSync(join(sibling, "devices.json"), JSON.stringify(registry), { mode: 0o600 });
const app = createServer({ port: 0, hostname: "127.0.0.1", stateDir: f.stateDir, siblingStateDir: sibling, token: "t", tailscaleOwner: null, machines: false, sidecar: false });
try {
expect(app.context.services.devices.hasKey(gone.spki)).toBe(false);
} finally { app.stop(); }
} finally { rmSync(sibling, { recursive: true, force: true }); }
});

test("merging two dirs keeps the account's removal of a key over a revoke on the PC, with the later time", () => {
const [a, b] = [mkdtempSync(join(tmpdir(), "ovis-account-a-")), mkdtempSync(join(tmpdir(), "ovis-account-b-"))];
try {
const key = "a-removed-key";
writeFileSync(join(a, "account-tombstones.json"), JSON.stringify({ v: 1, tombstones: [{ key, account: ACCOUNT, record_id: "r1", removed_at: "2026-11-01T00:00:00.000Z" }] }));
writeFileSync(join(b, "account-tombstones.json"), JSON.stringify({ v: 1, tombstones: [{ key, account: "", record_id: "local", removed_at: "2026-11-02T00:00:00.000Z" }] }));
expect(mergeAccountTombstones(a, b)).toBeNull();
for (const dir of [a, b]) {
const kept = JSON.parse(readFileSync(join(dir, "account-tombstones.json"), "utf8")) as { tombstones: { record_id: string; removed_at: string }[] };
expect(kept.tombstones.map((t) => [t.record_id, t.removed_at])).toEqual([["r1", "2026-11-02T00:00:00.000Z"]]);
}
} finally { for (const dir of [a, b]) rmSync(dir, { recursive: true, force: true }); }
});

test("merging two dirs rewrites a dir whose tombstone for the key is the PC's revoke at the same time as the account's removal", () => {
const [a, b] = [mkdtempSync(join(tmpdir(), "ovis-account-a-")), mkdtempSync(join(tmpdir(), "ovis-account-b-"))];
try {
const key = "a-removed-key";
const at = "2026-11-02T00:00:00.000Z";
writeFileSync(join(a, "account-tombstones.json"), JSON.stringify({ v: 1, tombstones: [{ key, account: "", record_id: "local", removed_at: at }] }));
writeFileSync(join(b, "account-tombstones.json"), JSON.stringify({ v: 1, tombstones: [{ key, account: ACCOUNT, record_id: "r1", removed_at: at }] }));
expect(mergeAccountTombstones(a, b)).toBeNull();
for (const dir of [a, b]) {
const kept = JSON.parse(readFileSync(join(dir, "account-tombstones.json"), "utf8")) as { tombstones: { record_id: string }[] };
expect(kept.tombstones.map((t) => t.record_id)).toEqual(["r1"]);
}
} finally { for (const dir of [a, b]) rmSync(dir, { recursive: true, force: true }); }
});


test("a watching member leaves the account on a PC with a remove signed by its own key", async () => {
const f = fixture();
const { key, link } = await member(f);
Expand Down
70 changes: 70 additions & 0 deletions server/device-access.contract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1180,3 +1180,73 @@ it("a script's first pairing on a fresh install is kept at once, so its token st
rmSync(stateDir, { recursive: true, force: true });
}
});

it("a PC in no account lets a revoked key pair again with a code started through the relay, which clears its tombstone", async () => {
const stateDir = mkdtempSync(join(tmpdir(), "ovis-relay-repair-"));
try {
const store = new DeviceStore(stateDir);
const keys = await crypto.subtle.generateKey({ name: "ECDSA", namedCurve: "P-256" }, true, ["sign", "verify"]);
const publicKey = Buffer.from(await crypto.subtle.exportKey("spki", keys.publicKey)).toString("base64url");
const first = store.pairRelay(store.startPairing().code, "Relay phone", publicKey);
if (!first) throw new Error("Expected relay enrollment");
store.revoke(first.device.id);
expect(store.isTombstoned(publicKey)).toBe(true);
const again = store.pairRelay(store.startPairing(Date.now(), { remote: true, startedBy: null, memberRequired: false }).code, "Relay phone", publicKey);
expect(again?.device.label).toBe("Relay phone");
expect(new DeviceStore(stateDir).isTombstoned(publicKey)).toBe(false);
} finally {
rmSync(stateDir, { recursive: true, force: true });
}
});

it("a PC in no account still refuses, with a code started through the relay, a key its account removed", async () => {
const stateDir = mkdtempSync(join(tmpdir(), "ovis-relay-account-removed-"));
try {
const keys = await crypto.subtle.generateKey({ name: "ECDSA", namedCurve: "P-256" }, true, ["sign", "verify"]);
const publicKey = Buffer.from(await crypto.subtle.exportKey("spki", keys.publicKey)).toString("base64url");
writeFileSync(join(stateDir, "account-tombstones.json"), JSON.stringify({ v: 1, tombstones: [{ key: publicKey, account: "acct_1", record_id: "r1", removed_at: "2026-11-01T00:00:00.000Z" }] }), { mode: 0o600 });
const store = new DeviceStore(stateDir);
expect(store.pairRelay(store.startPairing(Date.now(), { remote: true, startedBy: null, memberRequired: false }).code, "Phone", publicKey)).toBeNull();
expect(store.isTombstoned(publicKey)).toBe(true);
} finally {
rmSync(stateDir, { recursive: true, force: true });
}
});

it("a PC in no account whose tombstone file will not parse refuses a code started through the relay", async () => {
const stateDir = mkdtempSync(join(tmpdir(), "ovis-relay-tombstones-broken-"));
try {
writeFileSync(join(stateDir, "account-tombstones.json"), "{broken", { mode: 0o600 });
const store = new DeviceStore(stateDir);
const keys = await crypto.subtle.generateKey({ name: "ECDSA", namedCurve: "P-256" }, true, ["sign", "verify"]);
const publicKey = Buffer.from(await crypto.subtle.exportKey("spki", keys.publicKey)).toString("base64url");
expect(store.pairRelay(store.startPairing(Date.now(), { remote: true, startedBy: null, memberRequired: false }).code, "Phone", publicKey)).toBeNull();
} finally {
rmSync(stateDir, { recursive: true, force: true });
}
});

it("a key re-paired on a PC in no account with a code another device started stays when its tombstone comes back", async () => {
const stateDir = mkdtempSync(join(tmpdir(), "ovis-relay-repair-starter-"));
try {
const store = new DeviceStore(stateDir);
const fresh = async () => {
const keys = await crypto.subtle.generateKey({ name: "ECDSA", namedCurve: "P-256" }, true, ["sign", "verify"]);
return Buffer.from(await crypto.subtle.exportKey("spki", keys.publicKey)).toString("base64url");
};
const [starterKey, publicKey] = [await fresh(), await fresh()];
const starter = store.pairRelay(store.startPairing().code, "Laptop", starterKey);
const first = store.pairRelay(store.startPairing().code, "Phone", publicKey);
if (!starter || !first) throw new Error("Expected relay enrollment");
store.revoke(first.device.id);
const tombstones = readFileSync(join(stateDir, "account-tombstones.json"));
// a second later, as a person pairing again would be
const later = Date.now() + 1_000;
expect(store.pairRelay(store.startPairing(later, { remote: true, startedBy: starter.device.id, memberRequired: false }).code, "Phone", publicKey, later)).not.toBeNull();
// the revoke's tombstone back, as a stop before it was cleared or a sibling dir's merge leaves it
writeFileSync(join(stateDir, "account-tombstones.json"), tombstones, { mode: 0o600 });
expect(new DeviceStore(stateDir).hasKey(publicKey)).toBe(true);
} finally {
rmSync(stateDir, { recursive: true, force: true });
}
});
31 changes: 23 additions & 8 deletions server/devices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,11 +206,16 @@ export function mergeAccountTombstones(active: string, sibling: string): string
const merged = new Map<string, AccountTombstone>();
for (const tombstone of [...read.flatMap((list) => list ?? []), ...pending]) {
const kept = merged.get(tombstone.key);
if (!kept || tombstone.removed_at > kept.removed_at) merged.set(tombstone.key, tombstone);
if (!kept) { merged.set(tombstone.key, tombstone); continue; }
// the account's removal outranks a revoke on the PC (only it keeps a relay code out), with the later of the two times
const local = (t: AccountTombstone) => t.record_id === "local";
const winner = local(kept) && !local(tombstone) ? tombstone : !local(kept) && local(tombstone) ? kept : tombstone.removed_at > kept.removed_at ? tombstone : kept;
const removedAt = tombstone.removed_at > kept.removed_at ? tombstone.removed_at : kept.removed_at;
merged.set(tombstone.key, { ...winner, removed_at: removedAt });
}
const tombstones = [...merged.values()];
for (const [index, path] of paths.entries()) {
if ((read[index] ?? []).length !== tombstones.length || tombstones.some((t) => !read[index]?.some((own) => own.key === t.key && own.removed_at === t.removed_at))) {
if ((read[index] ?? []).length !== tombstones.length || tombstones.some((t) => !read[index]?.some((own) => own.key === t.key && own.removed_at === t.removed_at && own.record_id === t.record_id))) {
writePrivateState(path, { v: 1, tombstones });
}
}
Expand Down Expand Up @@ -479,14 +484,21 @@ export class DeviceStore {
// a code started through the relay is not a pairing made on this PC (docs/account.md, rule 5)
const remote = this.pending?.remote ?? false;
const startedBy = this.pending?.startedBy ?? null;
// a code started through the relay is not a pairing made on this PC; a member's code goes with the member
if ((remote && (this.accountError !== null || this.isTombstoned(publicKey))) || (this.pending?.memberRequired && (startedBy === null || !this.isAccountApprover(startedBy)))) {
// a code started through the relay is not a pairing made on this PC; a member's code goes with the member.
// On a PC in no account such a code may bring back a key this PC revoked itself, as before account linking;
// never one the account removed
const accountless = this.account() === null;
const refusedKey = accountless ? this.tombstones.some((t) => t.key === publicKey && t.record_id !== "local") : this.isTombstoned(publicKey);
if ((remote && (this.accountError !== null || refusedKey)) || (this.pending?.memberRequired && (startedBy === null || !this.isAccountApprover(startedBy)))) {
this.noteRelayFailure("code", now);
return null;
}
// a device let in by a code started through the relay is its starter's addition, not a pairing made on this PC (removeAccountKey)
const paired = this.enrollRelay(label, publicKey, now, true, startedBy ?? undefined);
if (!remote) this.clearTombstone(publicKey);
// a device let in by a code started through the relay is its starter's addition, not a pairing made on this PC (removeAccountKey);
// one that brings back a key this PC revoked itself (on a PC in no account) undoes that revoke, and is the PC's own
// pairing, so a tombstone that comes back (a sibling dir, a stop before it is cleared) takes it only if newer
const undoesRevoke = remote && accountless && this.isTombstoned(publicKey);
const paired = this.enrollRelay(label, publicKey, now, true, undoesRevoke ? undefined : startedBy ?? undefined);
if (!remote || accountless) this.clearTombstone(publicKey);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
return paired;
}

Expand Down Expand Up @@ -774,7 +786,10 @@ export class DeviceStore {
if (this.devices.some((d) => d.relay_public_key === key && d.added_by === undefined && d.created_at > record.issuedAt)) return false;
const recordId = record.id;
const fresh = !this.isTombstoned(key);
if (fresh) this.writeTombstones([...this.tombstones, { key, account, record_id: recordId, removed_at: new Date(now).toISOString() }]);
const removal = { key, account, record_id: recordId, removed_at: new Date(now).toISOString() };
if (fresh) this.writeTombstones([...this.tombstones, removal]);
// a key this PC revoked first becomes the account's removal, so a code started through the relay cannot bring it back
else if (this.tombstones.some((t) => t.key === key && t.record_id === "local")) this.writeTombstones(this.tombstones.map((t) => t.key === key ? removal : t));
const ids = this.devices.filter((d) => d.relay_public_key === key).map((d) => d.id);
for (const id of ids) this.revoke(id, record.by);
return fresh || ids.length > 0;
Expand Down
Loading