diff --git a/extension/src/service-worker/__tests__/org-write.test.ts b/extension/src/service-worker/__tests__/org-write.test.ts index 1e976c4..a09622f 100644 --- a/extension/src/service-worker/__tests__/org-write.test.ts +++ b/extension/src/service-worker/__tests__/org-write.test.ts @@ -72,6 +72,11 @@ const FILTERED_MANIFEST: OrgManifest = { // Fake encrypted manifest bytes returned by host.readFile('manifest.enc'). const FAKE_ENC_MANIFEST = new Uint8Array([0xaa, 0xbb, 0xcc]); +// The id new_item_id() mints for a new org item. C1 fix: handleOrgAddItem ignores +// the popup's item.id (which is '' for NEW items) and mints a fresh id, so the blob +// path + manifest entry never collide on the empty id. +const MINTED_ID = 'abcd1234abcd1234'; + const NEW_ITEM: Item = { id: 'item-new-001', title: 'New Login', @@ -119,6 +124,8 @@ function makeW() { ), org_manifest_encrypt: vi.fn().mockReturnValue(new Uint8Array([1, 2, 3])), item_encrypt: vi.fn().mockReturnValue(new Uint8Array([4, 5, 6])), + // new_item_id mints a fresh 16-hex id for new items (C1 fix). + new_item_id: vi.fn().mockReturnValue(MINTED_ID), // org_manifest_decrypt_filtered is called after every write to re-sync state.manifest. // Default returns FILTERED_MANIFEST; override per-test to assert specific post-write views. org_manifest_decrypt_filtered: vi.fn().mockReturnValue(FILTERED_MANIFEST), @@ -190,7 +197,8 @@ describe('org write handlers', () => { { signingPubOpenssh: string }, ]; const paths = files.map(f => f.path).sort(); - expect(paths).toEqual([`items/prod-infra/${NEW_ITEM.id}.enc`, 'manifest.enc'].sort()); + // Path uses the MINTED id (C1 fix), not the popup-supplied item.id. + expect(paths).toEqual([`items/prod-infra/${MINTED_ID}.enc`, 'manifest.enc'].sort()); expect(signer.signingPubOpenssh).toBe('ssh-ed25519 AAAA test'); }); @@ -215,7 +223,7 @@ describe('org write handlers', () => { expect(collections.has('secret-ops')).toBe(true); // 2 original entries + 1 new one expect(written.entries).toHaveLength(3); - expect(written.entries.find(e => e.id === NEW_ITEM.id)?.collection).toBe('prod-infra'); + expect(written.entries.find(e => e.id === MINTED_ID)?.collection).toBe('prod-infra'); }); // Test 4: update mutates the matching entry and dual-writes @@ -372,4 +380,82 @@ describe('org write handlers', () => { expect(grants).toEqual(['prod-infra']); expect(fakeState.manifest).toBe(FILTERED_AFTER_DELETE); }); + + // Test 10 (C1 regression): the popup sends id:'' for NEW items. The handler MUST + // mint a real id — never write items//.enc (which would overwrite the + // previously-added item in the same collection) or push a duplicate empty-id entry. + it('C1: org_add_item with empty item.id mints a real id for BOTH the blob path and the manifest entry', async () => { + const w = makeW(); + const fakeState = makeFakeState(); + await switchToOrg(fakeState, w); + + const newWithEmptyId: Item = { ...NEW_ITEM, id: '' }; + const result = await handleOrgAddItem({ collection: 'prod-infra', item: newWithEmptyId }, w); + expect(result).toEqual({ ok: true }); + expect(w.new_item_id).toHaveBeenCalled(); + + // Blob path must use the minted id — NOT items/prod-infra/.enc. + const commitSigned = fakeState.host.commitSigned as ReturnType; + const [files] = commitSigned.mock.calls[0] as [Array<{ path: string }>]; + const itemPath = files.map(f => f.path).find(p => p.startsWith('items/')); + expect(itemPath).toBe(`items/prod-infra/${MINTED_ID}.enc`); + expect(itemPath).not.toBe('items/prod-infra/.enc'); + + // Manifest entry must carry the minted id; no empty-id entry may exist. + const manifestEncrypt = w.org_manifest_encrypt as ReturnType; + const [, jsonArg] = manifestEncrypt.mock.calls[0] as [unknown, string]; + const written = JSON.parse(jsonArg) as OrgManifest; + expect(written.entries.some(e => e.id === MINTED_ID && e.collection === 'prod-infra')).toBe(true); + expect(written.entries.some(e => e.id === '')).toBe(false); + + // The ENCRYPTED item must also carry the minted id (item_encrypt gets withId). + const itemEncrypt = w.item_encrypt as ReturnType; + const [, itemJson] = itemEncrypt.mock.calls[0] as [unknown, string]; + expect((JSON.parse(itemJson) as Item).id).toBe(MINTED_ID); + }); + + // Test 11 (m1): update parity with add — an entry in an ungranted collection is refused. + it('org_update_item: entry in an ungranted collection → collection_not_granted; commitSigned not called', async () => { + const w = makeW(); + const fakeState = makeFakeState(); // granted prod-infra only + await switchToOrg(fakeState, w); + + // item-s1 lives in secret-ops (ungranted). + const result = await handleOrgUpdateItem({ id: 'item-s1', item: { ...UPDATED_ITEM, id: 'item-s1' } }, w); + + expect(result).toEqual({ ok: false, error: 'collection_not_granted' }); + expect(fakeState.host.commitSigned as ReturnType).not.toHaveBeenCalled(); + }); + + // Test 12 (m1): delete parity with add — an entry in an ungranted collection is refused. + it('org_delete_item: entry in an ungranted collection → collection_not_granted; commitSigned not called', async () => { + const w = makeW(); + const fakeState = makeFakeState(); + await switchToOrg(fakeState, w); + + const result = await handleOrgDeleteItem({ id: 'item-s1' }, w); + + expect(result).toEqual({ ok: false, error: 'collection_not_granted' }); + expect(fakeState.host.commitSigned as ReturnType).not.toHaveBeenCalled(); + }); + + // Test 13 (m4): a device name containing CR/LF/<> must be sanitized so the signed + // commit object's `author NAME …` line cannot be malformed/injected. + it('m4: buildSigner sanitizes a device name with newline / < / >', async () => { + const w = makeW(); + w.get_device_info.mockReturnValue({ + name: 'evil\nname ', + signing_public_key: 'ssh-ed25519 AAAA test', + deploy_public_key: 'ssh-ed25519 BBBB test', + }); + const fakeState = makeFakeState(); + await switchToOrg(fakeState, w); + + await handleOrgAddItem({ collection: 'prod-infra', item: NEW_ITEM }, w); + + const commitSigned = fakeState.host.commitSigned as ReturnType; + const [, , signer] = commitSigned.mock.calls[0] as [unknown, string, { author: { name: string; email: string } }]; + expect(signer.author.name).not.toMatch(/[\r\n<>]/); + expect(signer.author.email).toMatch(/^[A-Za-z0-9._-]+@relicario\.device$/); + }); }); diff --git a/extension/src/service-worker/router/org-handlers.ts b/extension/src/service-worker/router/org-handlers.ts index 725148e..930d548 100644 --- a/extension/src/service-worker/router/org-handlers.ts +++ b/extension/src/service-worker/router/org-handlers.ts @@ -120,8 +120,14 @@ function buildSigner(w: WasmModule): CommitSigner | null { deploy_public_key: string; } | null; if (!info) return null; + // Sanitize the device name: CR/LF/<> would malform the commit object's + // `author NAME …` line. The identity is cosmetic to the org hook (it + // matches on the signing-key fingerprint, not the text), so a sanitized + // display name + a safe email local-part is sufficient. + const name = info.name.replace(/[\r\n<>]/g, ' ').trim() || 'relicario-device'; + const emailLocal = name.replace(/[^A-Za-z0-9._-]/g, '-') || 'device'; return { - author: { name: info.name, email: `${info.name}@relicario.device` }, + author: { name, email: `${emailLocal}@relicario.device` }, signingPubOpenssh: info.signing_public_key, rawSign: (b: Uint8Array) => base64ToUint8Array((w.sign_for_git(b) as { signature: string }).signature), }; @@ -148,22 +154,28 @@ export async function handleOrgAddItem( // LANDMINE: decrypt FULL (unfiltered) manifest — never use state.manifest. const enc = await state.host.readFile('manifest.enc'); const full = w.org_manifest_decrypt(state.handle, enc) as OrgManifest; + // The popup sends id:'' for NEW items; the personal add_item handler mints the + // id via new_item_id() and the org path must do the same. Using item.id + // verbatim would write every new item to items//.enc and OVERWRITE + // the previous one (blob data loss + duplicate empty-id manifest entries). + const id = w.new_item_id() as string; + const withId: Item = { ...item, id }; full.entries.push({ - id: item.id, - type: item.type, - title: item.title, - tags: item.tags, - modified: item.modified, + id, + type: withId.type, + title: withId.title, + tags: withId.tags, + modified: withId.modified, collection, }); const manifestBytes = w.org_manifest_encrypt(state.handle, JSON.stringify(full)) as Uint8Array; - const itemBytes = w.item_encrypt(state.handle, JSON.stringify(item)) as Uint8Array; + const itemBytes = w.item_encrypt(state.handle, JSON.stringify(withId)) as Uint8Array; await state.host.commitSigned( [ - { path: `items/${collection}/${item.id}.enc`, content: itemBytes }, + { path: `items/${collection}/${id}.enc`, content: itemBytes }, { path: 'manifest.enc', content: manifestBytes }, ], - `org add ${item.type} to ${collection}`, + `org add ${withId.type} to ${collection}`, signer, ); // Re-sync the cached grant-filtered manifest so org_list_items reflects this write.