fix(ext): enforce org read-only in detail/drawer + context-aware vault drawer + escape switcher options

This commit is contained in:
adlee-was-taken
2026-06-27 12:52:36 -04:00
parent 59b0fcd985
commit cc4d4e8f36
12 changed files with 181 additions and 28 deletions

View File

@@ -117,6 +117,11 @@ export function openOrgAddForm(onAdded?: (orgId: string) => void): void {
memberId: (data.get('memberId') as string).trim(),
};
if (!config.orgId) {
errorSlot.textContent = 'Org ID is required';
return;
}
const resp = await sendMessage({ type: 'org_add_config', config });
if (resp.ok) {
close();

View File

@@ -7,7 +7,7 @@
// disable writes.
import type { OrgConfigSummary, OrgManifestEntry, ManifestEntry, ItemId, Collection } from '../../shared/types';
import { sendMessage, setState, navigate, getState } from '../../shared/state';
import { sendMessage, setState, navigate, getState, escapeHtml } from '../../shared/state';
import { messageForList, normalizeOrgEntries } from '../../shared/org-context';
import { openOrgAddForm } from './org-add-form';
@@ -33,7 +33,7 @@ export async function renderOrgSwitcher(host: HTMLElement): Promise<void> {
`<option value="personal"${currentContext === 'personal' ? ' selected' : ''}>Personal</option>`;
for (const org of orgConfigs) {
const sel = currentContext === org.orgId ? ' selected' : '';
optionsHtml += `<option value="${org.orgId}"${sel}>${org.displayName}</option>`;
optionsHtml += `<option value="${escapeHtml(org.orgId)}"${sel}>${escapeHtml(org.displayName)}</option>`;
}
host.innerHTML =
`<select class="org-switcher" aria-label="Vault context">${optionsHtml}</select>` +
@@ -62,7 +62,13 @@ export async function renderOrgSwitcher(host: HTMLElement): Promise<void> {
existing?.remove();
}
// Load the new context's entries into shared state before navigating.
// Fire the collection fetch immediately so it overlaps with the list fetch
// (both are independent after org_switch above). We await them sequentially
// below so setState({ entries }) still happens before setState({ orgCollections }).
const colPromise = data.context !== 'personal'
? sendMessage({ type: 'org_list_collections' })
: null;
// messageForList() reads the just-set orgContext so it picks the right SW
// message (list_items for personal, org_list_items for org). Both popup's
// renderItemList and vault's renderListPane read state.entries, so updating
@@ -81,8 +87,8 @@ export async function renderOrgSwitcher(host: HTMLElement): Promise<void> {
if (data.context === 'personal') {
setState({ orgCollections: [], collectionFilter: undefined });
} else {
const colResp = await sendMessage({ type: 'org_list_collections' });
if (colResp.ok) setState({ orgCollections: colResp.data as Collection[] });
const colResp = colPromise ? await colPromise : null;
if (colResp?.ok) setState({ orgCollections: colResp.data as Collection[] });
}
navigate('list', {});

View File

@@ -0,0 +1,101 @@
// Regression test: in org context the detail view must NOT render edit or
// trash buttons (Finding #1 from the whole-branch code review).
import { beforeEach, describe, expect, it, vi } from 'vitest';
// Mock shared/org-context so currentContext() returns a non-personal org.
vi.mock('../../../../shared/org-context', () => ({
currentContext: vi.fn(() => 'acme-corp'),
messageForGet: vi.fn((id: string) => ({ type: 'org_get_item', id })),
messageForList: vi.fn(() => ({ type: 'org_list_items' })),
normalizeOrgEntries: vi.fn((e: unknown[]) => e),
}));
vi.mock('../../../../shared/state', async () => {
const navigate = vi.fn();
const setState = vi.fn();
const sendMessage = vi.fn();
const getState = vi.fn(() => ({
view: 'detail', entries: [], selectedId: 'abc123', selectedItem: null,
selectedIndex: 0, searchQuery: '', activeGroup: null, error: null,
loading: false, capturedTabId: null, capturedUrl: '', newType: null,
orgContext: 'acme-corp', orgOffline: false,
}));
const escapeHtml = (s: string) =>
s.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;')
.replace(/"/g, '&quot;').replace(/'/g, '&#39;');
return {
navigate, setState, sendMessage, getState, escapeHtml,
popOutToTab: vi.fn(), isInTab: vi.fn(() => false), openVaultTab: vi.fn(),
};
});
vi.mock('../../../../setup/setup-helpers', () => ({
scheduleRate: vi.fn(),
STRENGTH_LABELS: {},
entropyText: vi.fn(() => ''),
}));
import { renderDetail } from '../login';
import type { Item } from '../../../../shared/types';
const ORG_LOGIN_ITEM: Item = {
id: 'aabbccdd00000001',
title: 'Acme Intranet',
type: 'login',
tags: [],
favorite: false,
created: 0,
modified: 0,
trashed_at: undefined,
notes: undefined,
group: undefined,
core: {
type: 'login',
username: 'alice',
password: 's3cr3t',
url: 'https://intranet.acme.com',
totp: undefined,
},
sections: [],
attachments: [],
field_history: {},
};
describe('login renderDetail — org context read-only gate', () => {
beforeEach(() => {
document.body.innerHTML = '<div id="app"></div>';
// Minimal chrome stub for wireAttachmentsDisclosure + wireFieldHandlers
(globalThis as Record<string, unknown>).chrome = {
storage: {
local: {
get: vi.fn((_k: unknown, cb: (v: Record<string, unknown>) => void) => cb({})),
set: vi.fn((_o: unknown, cb?: () => void) => cb?.()),
},
},
runtime: { sendMessage: vi.fn() },
};
});
it('renders no edit or trash buttons in org context', async () => {
const app = document.getElementById('app')!;
await renderDetail(app, ORG_LOGIN_ITEM);
expect(document.getElementById('edit-btn')).toBeNull();
expect(document.getElementById('trash-btn')).toBeNull();
});
it('keeps the back button (read affordance) in org context', async () => {
const app = document.getElementById('app')!;
await renderDetail(app, ORG_LOGIN_ITEM);
expect(document.getElementById('back-btn')).not.toBeNull();
});
it('keeps the fill/autofill button (read affordance) in org context', async () => {
const app = document.getElementById('app')!;
await renderDetail(app, ORG_LOGIN_ITEM);
expect(document.getElementById('fill-btn')).not.toBeNull();
});
});

View File

@@ -2,6 +2,7 @@
/// Detail view has a styled card-silhouette signature block.
import { getState, setState, sendMessage, navigate, escapeHtml, popOutToTab } from '../../../shared/state';
import { currentContext } from '../../../shared/org-context';
import { renderFormHeader } from '../form-header';
import { REQUIRED_PILL_HTML } from '../../../shared/glyphs';
import type { Item, ItemId, ManifestEntry, CardKind, Section, AttachmentRef } from '../../../shared/types';
@@ -58,6 +59,7 @@ function formatExpiry(e: { month: number; year: number } | undefined): string {
export async function renderDetail(app: HTMLElement, item: Item): Promise<void> {
if (item.core.type !== 'card') return;
const isPersonal = currentContext() === 'personal';
const c = item.core;
const number = c.number ?? '';
const brand = brandFromNumber(number);
@@ -94,8 +96,8 @@ export async function renderDetail(app: HTMLElement, item: Item): Promise<void>
${renderAttachmentsDisclosure({ itemId: item.id, attachments: item.attachments, mode: 'view' })}
<div class="form-actions" style="margin-top:14px;">
<button class="btn" id="back-btn">back</button>
<button class="btn" id="edit-btn">edit</button>
<button class="btn danger" id="trash-btn">trash</button>
${isPersonal ? '<button class="btn" id="edit-btn">edit</button>' : ''}
${isPersonal ? '<button class="btn danger" id="trash-btn">trash</button>' : ''}
</div>
</div>
`;
@@ -127,8 +129,11 @@ export async function renderDetail(app: HTMLElement, item: Item): Promise<void>
}
switch (e.key) {
case 'Escape': teardown(); navigate('list'); break;
case 'e': teardown(); navigate('edit'); break;
case 'e':
if (!isPersonal) break;
teardown(); navigate('edit'); break;
case 'd':
if (!isPersonal) break;
e.preventDefault();
if (confirm(`Move "${item.title}" to trash?`)) {
teardown();

View File

@@ -3,6 +3,7 @@
/// Primary attachment is referenced by ID from the item's attachments array.
import { getState, setState, sendMessage, navigate, escapeHtml, popOutToTab } from '../../../shared/state';
import { currentContext } from '../../../shared/org-context';
import { renderFormHeader } from '../form-header';
import { REQUIRED_PILL_HTML, GLYPH_TYPE_DOCUMENT, GLYPH_PREVIEW } from '../../../shared/glyphs';
import type { Item, ItemId, ManifestEntry, Section, AttachmentRef } from '../../../shared/types';
@@ -264,6 +265,7 @@ async function saveDocument(
export async function renderDetail(app: HTMLElement, item: Item): Promise<void> {
teardown();
if (item.core.type !== 'document') return;
const isPersonal = currentContext() === 'personal';
const c = item.core;
const primaryRef = item.attachments.find((a) => a.id === c.primary_attachment);
if (!primaryRef) {
@@ -301,8 +303,8 @@ export async function renderDetail(app: HTMLElement, item: Item): Promise<void>
<div class="form-actions" style="margin-top:14px;">
<button class="btn" id="back-btn">back</button>
<button class="btn" id="edit-btn">edit</button>
<button class="btn danger" id="trash-btn">trash</button>
${isPersonal ? '<button class="btn" id="edit-btn">edit</button>' : ''}
${isPersonal ? '<button class="btn danger" id="trash-btn">trash</button>' : ''}
</div>
</div>
`;
@@ -381,8 +383,11 @@ export async function renderDetail(app: HTMLElement, item: Item): Promise<void>
}
switch (e.key) {
case 'Escape': teardown(); navigate('list'); break;
case 'e': teardown(); navigate('edit'); break;
case 'e':
if (!isPersonal) break;
teardown(); navigate('edit'); break;
case 'd':
if (!isPersonal) break;
e.preventDefault();
if (confirm(`Move "${item.title}" to trash?`)) {
teardown();

View File

@@ -2,6 +2,7 @@
/// Detail view shows a "profile card" signature block + plain rows.
import { getState, setState, sendMessage, navigate, escapeHtml, popOutToTab } from '../../../shared/state';
import { currentContext } from '../../../shared/org-context';
import { renderFormHeader } from '../form-header';
import { REQUIRED_PILL_HTML } from '../../../shared/glyphs';
import type { Item, ItemId, ManifestEntry, Section, AttachmentRef } from '../../../shared/types';
@@ -48,6 +49,7 @@ function formatDate(iso: string | undefined): string {
export async function renderDetail(app: HTMLElement, item: Item): Promise<void> {
if (item.core.type !== 'identity') return;
const isPersonal = currentContext() === 'personal';
const c = item.core;
const sigInner = `
<div style="display:flex;align-items:center;gap:12px;">
@@ -72,8 +74,8 @@ export async function renderDetail(app: HTMLElement, item: Item): Promise<void>
${renderAttachmentsDisclosure({ itemId: item.id, attachments: item.attachments, mode: 'view' })}
<div class="form-actions" style="margin-top:14px;">
<button class="btn" id="back-btn">back</button>
<button class="btn" id="edit-btn">edit</button>
<button class="btn danger" id="trash-btn">trash</button>
${isPersonal ? '<button class="btn" id="edit-btn">edit</button>' : ''}
${isPersonal ? '<button class="btn danger" id="trash-btn">trash</button>' : ''}
</div>
</div>
`;
@@ -103,8 +105,11 @@ export async function renderDetail(app: HTMLElement, item: Item): Promise<void>
}
switch (e.key) {
case 'Escape': teardown(); navigate('list'); break;
case 'e': teardown(); navigate('edit'); break;
case 'e':
if (!isPersonal) break;
teardown(); navigate('edit'); break;
case 'd':
if (!isPersonal) break;
e.preventDefault();
if (confirm(`Move "${item.title}" to trash?`)) {
teardown();

View File

@@ -3,6 +3,7 @@
/// since <textarea type="password"> isn't a thing.
import { getState, setState, sendMessage, navigate, escapeHtml, popOutToTab } from '../../../shared/state';
import { currentContext } from '../../../shared/org-context';
import { renderFormHeader } from '../form-header';
import { REQUIRED_PILL_HTML } from '../../../shared/glyphs';
import type { Item, ItemId, ManifestEntry, Section, AttachmentRef } from '../../../shared/types';
@@ -35,6 +36,7 @@ export function teardown(): void {
export async function renderDetail(app: HTMLElement, item: Item): Promise<void> {
if (item.core.type !== 'key') return;
const isPersonal = currentContext() === 'personal';
const c = item.core;
const sigInner = `
@@ -57,8 +59,8 @@ export async function renderDetail(app: HTMLElement, item: Item): Promise<void>
${renderAttachmentsDisclosure({ itemId: item.id, attachments: item.attachments, mode: 'view' })}
<div class="form-actions" style="margin-top:14px;">
<button class="btn" id="back-btn">back</button>
<button class="btn" id="edit-btn">edit</button>
<button class="btn danger" id="trash-btn">trash</button>
${isPersonal ? '<button class="btn" id="edit-btn">edit</button>' : ''}
${isPersonal ? '<button class="btn danger" id="trash-btn">trash</button>' : ''}
</div>
</div>
`;
@@ -88,12 +90,15 @@ export async function renderDetail(app: HTMLElement, item: Item): Promise<void>
}
switch (e.key) {
case 'Escape': teardown(); navigate('list'); break;
case 'e': teardown(); navigate('edit'); break;
case 'e':
if (!isPersonal) break;
teardown(); navigate('edit'); break;
case 'c':
e.preventDefault();
try { await navigator.clipboard.writeText(c.key_material); } catch { /* swallow */ }
break;
case 'd':
if (!isPersonal) break;
e.preventDefault();
if (confirm(`Move "${item.title}" to trash?`)) {
teardown();

View File

@@ -2,6 +2,7 @@
/// field helpers introduced in Slice 2.
import { getState, setState, sendMessage, navigate, escapeHtml, popOutToTab, isInTab } from '../../../shared/state';
import { currentContext } from '../../../shared/org-context';
import { renderFormHeader } from '../form-header';
import { REQUIRED_PILL_HTML } from '../../../shared/glyphs';
import type { Item, ItemId, LoginCore, ManifestEntry, Section, TotpConfig, AttachmentRef } from '../../../shared/types';
@@ -65,6 +66,7 @@ export function teardown(): void {
export async function renderDetail(app: HTMLElement, item: Item): Promise<void> {
if (item.core.type !== 'login') return;
const isPersonal = currentContext() === 'personal';
const core = item.core as LoginCore & { type: 'login' };
const password = core.password ?? '';
const username = core.username ?? '';
@@ -99,9 +101,9 @@ export async function renderDetail(app: HTMLElement, item: Item): Promise<void>
<div class="form-actions" style="margin-top:14px;">
<button class="btn" id="back-btn">back</button>
${Object.keys(item.field_history).length > 0 ? '<button class="btn" id="history-btn">view history</button>' : ''}
<button class="btn" id="edit-btn">edit</button>
${isPersonal ? '<button class="btn" id="edit-btn">edit</button>' : ''}
<button class="btn" id="fill-btn">autofill</button>
<button class="btn danger" id="trash-btn">trash</button>
${isPersonal ? '<button class="btn danger" id="trash-btn">trash</button>' : ''}
</div>
</div>
`;
@@ -190,11 +192,13 @@ export async function renderDetail(app: HTMLElement, item: Item): Promise<void>
}
case 'e':
if (!isPersonal) break;
teardown();
navigate('edit');
break;
case 'd':
if (!isPersonal) break;
e.preventDefault();
if (confirm(`Move "${item.title}" to trash?`)) {
teardown();

View File

@@ -2,6 +2,7 @@
/// detail view; the form is just a big <textarea>.
import { getState, setState, sendMessage, navigate, escapeHtml, popOutToTab, isInTab } from '../../../shared/state';
import { currentContext } from '../../../shared/org-context';
import { renderFormHeader } from '../form-header';
import { REQUIRED_PILL_HTML } from '../../../shared/glyphs';
import type { Item, ItemId, ManifestEntry, Section, AttachmentRef } from '../../../shared/types';
@@ -34,6 +35,7 @@ export function teardown(): void {
export async function renderDetail(app: HTMLElement, item: Item): Promise<void> {
if (item.core.type !== 'secure_note') return;
const isPersonal = currentContext() === 'personal';
const body = item.core.body ?? '';
const sigInner = `
@@ -50,8 +52,8 @@ export async function renderDetail(app: HTMLElement, item: Item): Promise<void>
${renderAttachmentsDisclosure({ itemId: item.id, attachments: item.attachments, mode: 'view' })}
<div class="form-actions" style="margin-top:14px;">
<button class="btn" id="back-btn">back</button>
<button class="btn" id="edit-btn">edit</button>
<button class="btn danger" id="trash-btn">trash</button>
${isPersonal ? '<button class="btn" id="edit-btn">edit</button>' : ''}
${isPersonal ? '<button class="btn danger" id="trash-btn">trash</button>' : ''}
</div>
</div>
`;
@@ -81,8 +83,11 @@ export async function renderDetail(app: HTMLElement, item: Item): Promise<void>
}
switch (e.key) {
case 'Escape': teardown(); navigate('list'); break;
case 'e': teardown(); navigate('edit'); break;
case 'e':
if (!isPersonal) break;
teardown(); navigate('edit'); break;
case 'd':
if (!isPersonal) break;
e.preventDefault();
if (confirm(`Move "${item.title}" to trash?`)) {
teardown();

View File

@@ -3,6 +3,7 @@
/// (TOTP vs Steam Guard) and a single secret input.
import { getState, setState, sendMessage, navigate, escapeHtml, popOutToTab } from '../../../shared/state';
import { currentContext } from '../../../shared/org-context';
import { renderFormHeader } from '../form-header';
import { REQUIRED_PILL_HTML } from '../../../shared/glyphs';
import type { Item, ItemId, ManifestEntry, Section, TotpKind, AttachmentRef } from '../../../shared/types';
@@ -52,6 +53,7 @@ export function teardown(): void {
export async function renderDetail(app: HTMLElement, item: Item): Promise<void> {
if (item.core.type !== 'totp') return;
const isPersonal = currentContext() === 'personal';
const c = item.core;
const secretB32 = base32Encode(new Uint8Array(c.config.secret));
const isSteam = c.config.kind === 'steam';
@@ -98,8 +100,8 @@ export async function renderDetail(app: HTMLElement, item: Item): Promise<void>
${renderAttachmentsDisclosure({ itemId: item.id, attachments: item.attachments, mode: 'view' })}
<div class="form-actions" style="margin-top:14px;">
<button class="btn" id="back-btn">back</button>
<button class="btn" id="edit-btn">edit</button>
<button class="btn danger" id="trash-btn">trash</button>
${isPersonal ? '<button class="btn" id="edit-btn">edit</button>' : ''}
${isPersonal ? '<button class="btn danger" id="trash-btn">trash</button>' : ''}
</div>
</div>
`;
@@ -139,7 +141,9 @@ export async function renderDetail(app: HTMLElement, item: Item): Promise<void>
}
switch (e.key) {
case 'Escape': teardown(); navigate('list'); break;
case 'e': teardown(); navigate('edit'); break;
case 'e':
if (!isPersonal) break;
teardown(); navigate('edit'); break;
case 't': {
// Copy the currently displayed rotating code.
const codeEl = document.getElementById('totp-code');
@@ -150,6 +154,7 @@ export async function renderDetail(app: HTMLElement, item: Item): Promise<void>
break;
}
case 'd':
if (!isPersonal) break;
e.preventDefault();
if (confirm(`Move "${item.title}" to trash?`)) {
teardown();

View File

@@ -7,6 +7,7 @@ import type { Item } from '../shared/types';
import {
type VaultController, type VaultState, type HashRoute, escapeHtml,
} from './vault-context';
import { currentContext, messageForGet } from '../shared/org-context';
export function openDrawer(): void {
document.getElementById('vault-drawer')?.classList.add('vault-drawer--open');
@@ -74,13 +75,14 @@ export function renderDrawer(ctx: VaultController, item: Item): void {
const drawer = document.getElementById('vault-drawer');
if (!drawer) return;
const isPersonal = currentContext() === 'personal';
const coreFields = getDrawerCoreFields(item);
drawer.innerHTML = `
<div class="vault-drawer__header">
<span class="vault-drawer__type-pill">${item.type.replace('_', ' ').toUpperCase()}</span>
<div class="vault-drawer__actions">
<button class="btn" id="drawer-edit-btn" style="font-size:11px;">edit</button>
${isPersonal ? '<button class="btn" id="drawer-edit-btn" style="font-size:11px;">edit</button>' : ''}
<button class="vault-drawer__close" id="drawer-close-btn" title="Close (Esc)">✕</button>
</div>
</div>
@@ -114,7 +116,7 @@ export function renderDrawer(ctx: VaultController, item: Item): void {
}
export async function selectItemForDrawer(ctx: VaultController, id: string): Promise<void> {
const resp = await ctx.sendMessage({ type: 'get_item', id });
const resp = await ctx.sendMessage(messageForGet(id));
if (!resp.ok) return;
const data = resp.data as { item: Item };
ctx.state.selectedId = id;

View File

@@ -14,6 +14,7 @@ import {
type VaultController, typeIcon, typeLabel, getFilteredEntries, escapeHtml,
} from './vault-context';
import { getState, setState } from '../shared/state';
import { currentContext } from '../shared/org-context';
const SEARCH_DEBOUNCE_MS = 80;
@@ -22,6 +23,10 @@ const SEARCH_DEBOUNCE_MS = 80;
// ---------------------------------------------------------------------------
export function renderSidebarShell(): string {
// currentContext() reads shared state which may not be registered in all
// environments (e.g. isolated unit tests). Default to personal (show all UI).
let isPersonal = true;
try { isPersonal = currentContext() === 'personal'; } catch { /* no state host */ }
return `
<div class="vault-sidebar">
<div class="vault-sidebar__header">
@@ -35,7 +40,7 @@ export function renderSidebarShell(): string {
<nav class="vault-sidebar__categories" id="vault-categories" aria-label="Item types"></nav>
<nav class="vault-sidebar__collections" id="vault-collections" aria-label="Collections"></nav>
<div class="vault-sidebar__nav">
<button class="vault-sidebar__nav-item vault-sidebar__nav-item--primary" data-nav="add" title="New item">+ new item</button>
${isPersonal ? '<button class="vault-sidebar__nav-item vault-sidebar__nav-item--primary" data-nav="add" title="New item">+ new item</button>' : ''}
<button class="vault-sidebar__nav-item" data-nav="trash" title="Trash">${GLYPH_TRASH} <span class="vault-sidebar__nav-label">trash</span></button>
<button class="vault-sidebar__nav-item" data-nav="devices" title="Devices">${GLYPH_DEVICES} <span class="vault-sidebar__nav-label">devices</span></button>
<button class="vault-sidebar__nav-item" data-nav="settings" title="Settings">${GLYPH_SETTINGS} <span class="vault-sidebar__nav-label">settings</span></button>