fix(ext): content-callable capture_save_login closes critical router gap
After Slice 4's router split, the capture prompt's Save button was silently failing on every site: content/capture.ts called four handlers (get_settings, get_item, update_item, add_item) that are all in POPUP_ONLY_TYPES, so the router rejected each with unauthorized_sender. Fix in two parts: Part A — get_settings: content scripts already have storage permission via the manifest, so read relicarioSettings directly from chrome.storage.local instead of round-tripping through the SW. Part B — new content-callable 'capture_save_login' message that consolidates what was previously three separate popup-only calls (get_item + update_item or add_item) into one SW-side operation. Content scripts no longer need to distinguish add vs update — the SW does that itself from the manifest. Security model (all enforced SW-side, never trusting content): - Origin is derived from sender.tab.url by the router. The payload contains only username + password; there is no way for content to influence which host the new/updated item binds to. - Update path re-verifies the existing item's core.url hostname matches senderHost before mutating. If the manifest icon_hint ever drifts from core.url, we return origin_mismatch rather than silently binding a password to the wrong origin. - Update mutates ONLY the password field + modified timestamp — never title, url, or any other core field. - Add path creates a new Login item whose title is senderHost and whose url is the sender's origin. Five new router tests cover: content-accept, popup-reject, update path rotates only the password, add path creates bound item, and origin_mismatch when the stored item's host disagrees with senderHost. Tests: 47 -> 52. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -14,6 +14,8 @@ vi.mock('../../vault', async (importOriginal) => {
|
||||
fetchAndDecryptItem: vi.fn(),
|
||||
fetchAndDecryptSettings: vi.fn(),
|
||||
encryptAndWriteSettings: vi.fn(),
|
||||
encryptAndWriteItem: vi.fn(),
|
||||
encryptAndWriteManifest: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
@@ -340,3 +342,151 @@ describe('isContent sender.id guard', () => {
|
||||
expect(res).toEqual({ ok: false, error: 'unauthorized_sender' });
|
||||
});
|
||||
});
|
||||
|
||||
// --- capture_save_login (content-callable, origin-bound) ---
|
||||
|
||||
describe('capture_save_login', () => {
|
||||
const EXISTING_ID = 'dddddddddddddddd';
|
||||
|
||||
function loginItem(url: string, username: string, password: string): Item {
|
||||
return {
|
||||
id: EXISTING_ID,
|
||||
title: 'Example',
|
||||
type: 'login',
|
||||
tags: [],
|
||||
favorite: false,
|
||||
created: 0,
|
||||
modified: 0,
|
||||
core: { type: 'login', username, password, url },
|
||||
sections: [],
|
||||
attachments: [],
|
||||
field_history: {},
|
||||
};
|
||||
}
|
||||
|
||||
function primeUnlocked(state: RouterState): void {
|
||||
vi.mocked(session.getCurrent).mockReturnValue({ free: () => {} } as never);
|
||||
state.gitHost = {} as never;
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.mocked(session.getCurrent).mockReset();
|
||||
vi.mocked(vault.fetchAndDecryptItem).mockReset();
|
||||
vi.mocked(vault.encryptAndWriteItem).mockReset();
|
||||
vi.mocked(vault.encryptAndWriteManifest).mockReset();
|
||||
vi.mocked(vault.encryptAndWriteItem).mockResolvedValue(undefined);
|
||||
vi.mocked(vault.encryptAndWriteManifest).mockResolvedValue(undefined);
|
||||
});
|
||||
|
||||
it('accepts capture_save_login from top-frame content', async () => {
|
||||
const state = makeState();
|
||||
primeUnlocked(state);
|
||||
const res = await route(
|
||||
{ type: 'capture_save_login', username: 'alice', password: 'hunter2' },
|
||||
state,
|
||||
makeContentSender('https://example.com/login'),
|
||||
);
|
||||
expect(res.ok).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects capture_save_login from popup', async () => {
|
||||
const state = makeState();
|
||||
primeUnlocked(state);
|
||||
const res = await route(
|
||||
{ type: 'capture_save_login', username: 'alice', password: 'hunter2' },
|
||||
state,
|
||||
makePopupSender(),
|
||||
);
|
||||
expect(res).toEqual({ ok: false, error: 'unauthorized_sender' });
|
||||
});
|
||||
|
||||
it('update path: existing (host, username) match rotates the password', async () => {
|
||||
const state = makeState();
|
||||
primeUnlocked(state);
|
||||
// Seed manifest with a login for example.com.
|
||||
state.manifest = {
|
||||
schema_version: 2,
|
||||
items: {
|
||||
[EXISTING_ID]: {
|
||||
id: EXISTING_ID, type: 'login', title: 'Example',
|
||||
tags: [], favorite: false, icon_hint: 'example.com',
|
||||
modified: 0, attachment_summaries: [],
|
||||
},
|
||||
},
|
||||
};
|
||||
vi.mocked(vault.fetchAndDecryptItem).mockResolvedValue(
|
||||
loginItem('https://example.com/', 'alice', 'oldpass'),
|
||||
);
|
||||
|
||||
const res = await route(
|
||||
{ type: 'capture_save_login', username: 'alice', password: 'newpass' },
|
||||
state,
|
||||
makeContentSender('https://example.com/login'),
|
||||
);
|
||||
expect(res).toMatchObject({ ok: true, data: { action: 'updated', id: EXISTING_ID } });
|
||||
// Verify write was invoked with a core whose password is the new one.
|
||||
expect(vault.encryptAndWriteItem).toHaveBeenCalledTimes(1);
|
||||
const writtenItem = vi.mocked(vault.encryptAndWriteItem).mock.calls[0][3];
|
||||
expect(writtenItem.id).toBe(EXISTING_ID);
|
||||
if (writtenItem.core.type !== 'login') throw new Error('expected login core');
|
||||
expect(writtenItem.core.password).toBe('newpass');
|
||||
expect(writtenItem.core.username).toBe('alice');
|
||||
});
|
||||
|
||||
it('add path: no match creates a new item bound to senderHost', async () => {
|
||||
const state = makeState();
|
||||
primeUnlocked(state);
|
||||
// Empty manifest — no candidates.
|
||||
state.manifest = { schema_version: 2, items: {} };
|
||||
|
||||
const res = await route(
|
||||
{ type: 'capture_save_login', username: 'bob', password: 's3cret' },
|
||||
state,
|
||||
makeContentSender('https://example.com/signup'),
|
||||
);
|
||||
expect(res.ok).toBe(true);
|
||||
if (res.ok) {
|
||||
const data = res.data as { action: string; id: string };
|
||||
expect(data.action).toBe('added');
|
||||
expect(data.id).toBe('fakeitemid0000ab'); // from stub new_item_id()
|
||||
}
|
||||
expect(vault.encryptAndWriteItem).toHaveBeenCalledTimes(1);
|
||||
const newItem = vi.mocked(vault.encryptAndWriteItem).mock.calls[0][3];
|
||||
expect(newItem.title).toBe('example.com');
|
||||
if (newItem.core.type !== 'login') throw new Error('expected login core');
|
||||
expect(newItem.core.url).toBe('https://example.com');
|
||||
expect(newItem.core.username).toBe('bob');
|
||||
expect(newItem.core.password).toBe('s3cret');
|
||||
// Manifest entry should have been added too.
|
||||
expect(state.manifest!.items['fakeitemid0000ab']).toBeDefined();
|
||||
});
|
||||
|
||||
it('origin_mismatch when existing item for same username has a different host', async () => {
|
||||
const state = makeState();
|
||||
primeUnlocked(state);
|
||||
// Manifest says there's a match for example.com (icon_hint), but the
|
||||
// underlying item actually belongs to github.com — defense-in-depth
|
||||
// check should reject.
|
||||
state.manifest = {
|
||||
schema_version: 2,
|
||||
items: {
|
||||
[EXISTING_ID]: {
|
||||
id: EXISTING_ID, type: 'login', title: 'Example',
|
||||
tags: [], favorite: false, icon_hint: 'example.com',
|
||||
modified: 0, attachment_summaries: [],
|
||||
},
|
||||
},
|
||||
};
|
||||
vi.mocked(vault.fetchAndDecryptItem).mockResolvedValue(
|
||||
loginItem('https://github.com/', 'alice', 'oldpass'),
|
||||
);
|
||||
|
||||
const res = await route(
|
||||
{ type: 'capture_save_login', username: 'alice', password: 'newpass' },
|
||||
state,
|
||||
makeContentSender('https://example.com/login'),
|
||||
);
|
||||
expect(res).toEqual({ ok: false, error: 'origin_mismatch' });
|
||||
expect(vault.encryptAndWriteItem).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
/// sender.tab !== undefined.
|
||||
|
||||
import type { ContentMessage, Response } from '../../shared/messages';
|
||||
import type { Manifest } from '../../shared/types';
|
||||
import type { Item, Manifest } from '../../shared/types';
|
||||
import type { GitHost } from '../git-host';
|
||||
import * as vault from '../vault';
|
||||
import * as session from '../session';
|
||||
@@ -13,6 +13,8 @@ import * as session from '../session';
|
||||
export interface ContentState {
|
||||
manifest: Manifest | null;
|
||||
gitHost: GitHost | null;
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
wasm: any;
|
||||
}
|
||||
|
||||
export async function handle(
|
||||
@@ -93,9 +95,95 @@ export async function handle(
|
||||
}
|
||||
return { ok: true };
|
||||
}
|
||||
|
||||
case 'capture_save_login': {
|
||||
const handle = session.getCurrent();
|
||||
if (!handle || !state.gitHost || !state.manifest) return { ok: false, error: 'vault_locked' };
|
||||
|
||||
// Look for an existing login for this origin + username. Origin is
|
||||
// always senderHost (derived from sender.tab.url by the router) — the
|
||||
// content script cannot influence which host we bind to.
|
||||
const candidates = vault.findByHostname(state.manifest, senderHost);
|
||||
for (const [id, entry] of candidates) {
|
||||
if (entry.type !== 'login') continue;
|
||||
const full = await vault.fetchAndDecryptItem(state.gitHost, handle, id);
|
||||
if (full.core.type !== 'login') continue;
|
||||
if (full.core.username === msg.username) {
|
||||
// Defense in depth: verify the existing item's own URL hostname
|
||||
// matches senderHost. If it doesn't (e.g. manifest icon_hint
|
||||
// drifted from core.url), refuse to mutate — updating here would
|
||||
// silently bind a password to the wrong origin.
|
||||
const existingHost = safeHostname(full.core.url ?? '');
|
||||
if (existingHost !== senderHost) return { ok: false, error: 'origin_mismatch' };
|
||||
|
||||
// Update only the password field + modified timestamp.
|
||||
const updated: Item = {
|
||||
...full,
|
||||
modified: Math.floor(Date.now() / 1000),
|
||||
core: { ...full.core, password: msg.password },
|
||||
};
|
||||
await vault.encryptAndWriteItem(state.gitHost, handle, id, updated, `capture: update ${existingHost}`);
|
||||
state.manifest.items[id] = itemToManifestEntry(updated);
|
||||
await vault.encryptAndWriteManifest(state.gitHost, handle, state.manifest, `manifest: update ${existingHost}`);
|
||||
return { ok: true, data: { action: 'updated', id } };
|
||||
}
|
||||
}
|
||||
|
||||
// No match → create a new Login item bound to senderHost. Title
|
||||
// defaults to the hostname; url is the sender's full origin when we
|
||||
// have it, otherwise derived from senderHost.
|
||||
const now = Math.floor(Date.now() / 1000);
|
||||
const newId = state.wasm.new_item_id();
|
||||
const senderOrigin = (() => {
|
||||
try { return sender.tab?.url ? new URL(sender.tab.url).origin : `https://${senderHost}`; }
|
||||
catch { return `https://${senderHost}`; }
|
||||
})();
|
||||
const item: Item = {
|
||||
id: newId,
|
||||
title: senderHost,
|
||||
type: 'login',
|
||||
tags: [],
|
||||
favorite: false,
|
||||
created: now,
|
||||
modified: now,
|
||||
core: {
|
||||
type: 'login',
|
||||
username: msg.username,
|
||||
password: msg.password,
|
||||
url: senderOrigin,
|
||||
},
|
||||
sections: [],
|
||||
attachments: [],
|
||||
field_history: {},
|
||||
};
|
||||
await vault.encryptAndWriteItem(state.gitHost, handle, newId, item, `capture: add ${senderHost}`);
|
||||
state.manifest.items[newId] = itemToManifestEntry(item);
|
||||
await vault.encryptAndWriteManifest(state.gitHost, handle, state.manifest, `manifest: add ${senderHost}`);
|
||||
return { ok: true, data: { action: 'added', id: newId } };
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// --- Manifest entry derivation (duplicated from popup-only for self-containment) ---
|
||||
|
||||
function itemToManifestEntry(item: Item) {
|
||||
return {
|
||||
id: item.id,
|
||||
type: item.type,
|
||||
title: item.title,
|
||||
tags: item.tags,
|
||||
favorite: item.favorite,
|
||||
group: item.group,
|
||||
icon_hint: (item.core.type === 'login' && item.core.url)
|
||||
? safeHostname(item.core.url) : undefined,
|
||||
modified: item.modified,
|
||||
trashed_at: item.trashed_at,
|
||||
attachment_summaries: item.attachments.map((a) => ({
|
||||
id: a.id, filename: a.filename, mime_type: a.mime_type, size: a.size,
|
||||
})),
|
||||
};
|
||||
}
|
||||
|
||||
async function loadDeviceSettings(): Promise<{ captureEnabled: boolean; captureStyle: 'bar' | 'toast' }> {
|
||||
const r = await chrome.storage.local.get('relicarioSettings');
|
||||
return (r.relicarioSettings as { captureEnabled: boolean; captureStyle: 'bar' | 'toast' })
|
||||
|
||||
Reference in New Issue
Block a user