diff --git a/.changeset/path-picker-show-hidden.md b/.changeset/path-picker-show-hidden.md new file mode 100644 index 00000000..49971527 --- /dev/null +++ b/.changeset/path-picker-show-hidden.md @@ -0,0 +1,29 @@ +--- +'aicodeman': patch +--- + +The filesystem path picker can show hidden files and folders, and the shared secret blocklist grew to make that safe. + +The picker behind Link Existing's "Browse" and the mobile keyboard's `Path` key +refused every path with a dot-prefixed segment, so `.github/workflows/ci.yml` +could not be selected and a hidden folder could not even be opened. It now has +the same `.*` toggle as the File Viewer, default OFF, per-device, and it applies +to both the listing and the preview endpoint (which re-resolves the path +independently). + +That filter was quietly doing security work. With every hidden path unreachable, +`isSensitivePath` never had to name the credentials that live in dot-directories, +because the picker's roots include Home. Lifting the filter removes that +accident, so the blocklist now covers them explicitly: SSH keys at any depth (not +only under `$HOME`), GPG keyrings, AWS/GCloud/Azure/Docker/Kubernetes +credentials, npm, Yarn, git, `gh`, netrc, PyPI, RubyGems, Cargo and Terraform +tokens, `.pgpass` and `.my.cnf`, and the Claude and Codeman agent credentials. +`~/.codeman/` and `~/.claude/` stay attachable as trees, since the publish skill +and the review-card loop read from them; only their secret-bearing members are +named. + +Blocked trees, sensitive files, root confinement and symlink-escape checks are +all unchanged and still apply with the toggle on: a hidden entry that resolves +to a secret is dropped from the listing, and opening it is refused. + +Follows #221. diff --git a/src/web/public/keyboard-accessory.js b/src/web/public/keyboard-accessory.js index 5fc3bb3c..6972008d 100644 --- a/src/web/public/keyboard-accessory.js +++ b/src/web/public/keyboard-accessory.js @@ -33,6 +33,12 @@ // Shared Filesystem Path Picker // ═══════════════════════════════════════════════════════════════ +// Per-device, and deliberately its own key rather than a shared "show hidden" +// preference with the File Viewer: that tree is confined to one workspace, while +// the picker browses Home and every configured root, so wanting dotfiles in a +// project does not imply wanting them in ~. +const PATH_PICKER_SHOW_HIDDEN_KEY = 'codeman:pathPickerShowHidden'; + const PathPicker = { overlay: null, _options: null, @@ -43,6 +49,7 @@ const PathPicker = { _previewOverlay: null, _previewRequestSequence: 0, _previewPreviousFocus: null, + _showHidden: false, /** * Open the lazy filesystem browser. @@ -53,6 +60,7 @@ const PathPicker = { this.close(false); this._options = options; this._selectedPath = ''; + this._showHidden = this._loadShowHidden(); this._previousFocus = document.activeElement; this._previousFocus?.blur?.(); @@ -74,6 +82,7 @@ const PathPicker = {
+
Loading...
@@ -100,6 +109,8 @@ const PathPicker = { if (current) this.select(current); }); overlay.querySelector('.path-picker-refresh').addEventListener('click', () => this.load()); + overlay.querySelector('.path-picker-hidden').addEventListener('click', () => this.toggleHidden()); + this._syncHiddenButton(); overlay.querySelector('.path-picker-up').addEventListener('click', () => { const parent = overlay.querySelector('.path-picker-up').dataset.parent; if (parent) this.load(parent); @@ -120,6 +131,38 @@ const PathPicker = { this.load(options.initialPath || ''); }, + _loadShowHidden() { + try { + return localStorage.getItem(PATH_PICKER_SHOW_HIDDEN_KEY) === '1'; + } catch { + return false; + } + }, + + _syncHiddenButton() { + const btn = this.overlay?.querySelector('.path-picker-hidden'); + if (!btn) return; + const label = this._showHidden ? 'Hide hidden files and folders' : 'Show hidden files and folders'; + btn.classList.toggle('active', this._showHidden); + btn.setAttribute('aria-pressed', this._showHidden ? 'true' : 'false'); + btn.setAttribute('title', label); + btn.setAttribute('aria-label', label); + }, + + toggleHidden() { + if (!this.overlay) return; + this._showHidden = !this._showHidden; + try { + localStorage.setItem(PATH_PICKER_SHOW_HIDDEN_KEY, this._showHidden ? '1' : '0'); + } catch {} + this._syncHiddenButton(); + // Reload where we are rather than resetting to the root. Turning the toggle + // OFF inside a hidden folder makes the current path unbrowsable again; the + // server answers 403 and load()'s catch falls back to the default root, + // which is the only place left to stand. + this.load(this.overlay.querySelector('.path-picker-current').textContent || ''); + }, + async load(path) { if (!this.overlay || !this._options) return; const loadSequence = ++this._loadSequence; @@ -131,6 +174,7 @@ const PathPicker = { const params = new URLSearchParams(); if (path) params.set('path', path); if (this._options.sessionId) params.set('sessionId', this._options.sessionId); + if (this._showHidden) params.set('showHidden', 'true'); try { const response = await fetch(`/api/filesystem/browse?${params.toString()}`); const result = await response.json(); @@ -248,6 +292,9 @@ const PathPicker = { const requestSequence = ++this._previewRequestSequence; const params = new URLSearchParams({ path: entry.path }); if (this._options?.sessionId) params.set('sessionId', this._options.sessionId); + // A hidden file is only reachable while the toggle is on, and the preview + // endpoint re-resolves the path independently, so it needs the flag too. + if (this._showHidden) params.set('showHidden', 'true'); const previewUrl = `/api/filesystem/preview?${params.toString()}`; const overlay = document.createElement('div'); diff --git a/src/web/public/styles.css b/src/web/public/styles.css index 62236f0f..dd9a34b9 100644 --- a/src/web/public/styles.css +++ b/src/web/public/styles.css @@ -11980,7 +11980,8 @@ body.touch-device.cjk-input-visible .main { } .path-picker-up, -.path-picker-refresh { +.path-picker-refresh, +.path-picker-hidden { flex: 0 0 38px; height: 38px; color: var(--text); @@ -11990,6 +11991,20 @@ body.touch-device.cjk-input-visible .main { cursor: pointer; } +/* Show-hidden toggle: a literal `.*` glyph rather than an icon, so its meaning + * (dot-prefixed files and folders) survives every skin and font stack. */ +.path-picker-hidden { + font-family: var(--font-mono, monospace); + font-size: 0.9rem; + font-weight: 700; + letter-spacing: -0.05em; +} + +.path-picker-hidden.active { + color: var(--accent); + border-color: var(--accent); +} + .path-picker-up:disabled { opacity: 0.35; cursor: default; diff --git a/src/web/routes/file-routes.ts b/src/web/routes/file-routes.ts index 4a444044..2246eaf1 100644 --- a/src/web/routes/file-routes.ts +++ b/src/web/routes/file-routes.ts @@ -315,11 +315,25 @@ function findMatchingPickerRoot(roots: FilesystemBrowseRoot[], candidate: string .sort((a, b) => b.path.length - a.path.length)[0]; } +/** + * Whether a path has a dot-prefixed segment anywhere below its browse root. + * + * Checked against the REALPATH, so a plainly-named symlink pointing into a + * hidden tree is caught too. Callers skip it when the request opts into hidden + * entries (`showHidden`), which is why the sensitive-path blocklist and the + * blocked-tree checks must stand on their own: with the toggle on, this is no + * longer the thing keeping `~/.config/gh/hosts.yml` out of reach. + */ function containsHiddenPickerSegment(root: string, candidate: string): boolean { const rel = relative(root, candidate); return rel !== '' && rel.split(sep).some((segment) => segment.startsWith('.')); } +/** Parses the picker's opt-in `showHidden` query flag (absent means off). */ +function wantsHiddenPickerEntries(showHidden?: string): boolean { + return showHidden === 'true'; +} + function getFilesystemPreviewKind(fileName: string): FilesystemPreviewKind | undefined { const extension = extname(fileName).slice(1).toLowerCase(); if (FILESYSTEM_IMAGE_PREVIEW_EXTENSIONS.has(extension)) return 'image'; @@ -431,7 +445,8 @@ async function resolveFilesystemPickerPath( ctx: SessionPort & ConfigPort, req: FastifyRequest, requestedPath: string | undefined, - sessionId?: string + sessionId?: string, + showHidden = false ): Promise { const roots = await resolveFilesystemPickerRoots(ctx, req, sessionId); if (roots.length === 0) { @@ -453,7 +468,7 @@ async function resolveFilesystemPickerPath( if (!matchingRoot) { throwFilesystemPickerError(403, ApiErrorCode.INVALID_INPUT, 'Path is outside the allowed browse roots'); } - if (containsHiddenPickerSegment(matchingRoot.path, resolvedPath)) { + if (!showHidden && containsHiddenPickerSegment(matchingRoot.path, resolvedPath)) { throwFilesystemPickerError(403, ApiErrorCode.INVALID_INPUT, 'Hidden paths are not available in the file picker'); } @@ -662,12 +677,14 @@ function inheritedHeaders(reply: { export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & EventPort & ConfigPort): void { // Lazy filesystem listing for the Link Existing and mobile input path pickers. app.get('/api/filesystem/browse', async (req, reply): Promise> => { - const { path: requestedPath, sessionId } = parseBody(FilesystemBrowseQuerySchema, req.query); + const { path: requestedPath, sessionId, showHidden } = parseBody(FilesystemBrowseQuerySchema, req.query); + const includeHidden = wantsHiddenPickerEntries(showHidden); const { candidatePath, resolvedPath, roots, matchingRoot, blockedTrees } = await resolveFilesystemPickerPath( ctx, req, requestedPath, - sessionId + sessionId, + includeHidden ); if (isBlockedPickerPath(resolvedPath, blockedTrees, true)) { @@ -703,7 +720,7 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even const entries: FilesystemBrowseEntry[] = []; let truncated = false; for (const entry of dirEntries) { - if (entry.name.startsWith('.')) continue; + if (!includeHidden && entry.name.startsWith('.')) continue; if (entries.length >= FILESYSTEM_PICKER_ENTRY_LIMIT) { truncated = true; break; @@ -718,7 +735,8 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even } const targetRoot = findMatchingPickerRoot(roots, targetPath); - if (!targetRoot || containsHiddenPickerSegment(targetRoot.path, targetPath)) continue; + if (!targetRoot) continue; + if (!includeHidden && containsHiddenPickerSegment(targetRoot.path, targetPath)) continue; let type: FilesystemBrowseEntry['type']; let size: number | undefined; @@ -783,12 +801,13 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even // Inline preview for files selected through the root-confined filesystem picker. app.get('/api/filesystem/preview', { compress: false }, async (req, reply): Promise => { - const { path: requestedPath, sessionId } = parseBody(FilesystemPreviewQuerySchema, req.query); + const { path: requestedPath, sessionId, showHidden } = parseBody(FilesystemPreviewQuerySchema, req.query); const { candidatePath, resolvedPath, blockedTrees } = await resolveFilesystemPickerPath( ctx, req, requestedPath, - sessionId + sessionId, + wantsHiddenPickerEntries(showHidden) ); if (isBlockedPickerPath(resolvedPath, blockedTrees)) { throwFilesystemPickerError(403, ApiErrorCode.INVALID_INPUT, 'Access to this file is blocked'); diff --git a/src/web/schemas.ts b/src/web/schemas.ts index cf2d3d82..9f6b21e4 100644 --- a/src/web/schemas.ts +++ b/src/web/schemas.ts @@ -65,6 +65,14 @@ const filesystemPickerPathSchema = z }) .refine((p) => !p.split('/').includes('..'), { message: 'Path traversal is not allowed' }); +/** + * Opt-in flag for listing dot-prefixed entries in the path picker. Absent means + * off, so an old client keeps the previous behavior. It is a string rather than + * a boolean because it arrives as a query parameter; `'false'` is accepted (and + * means off) so a client can send the flag unconditionally. + */ +const showHiddenQuerySchema = z.enum(['true', 'false']).optional(); + /** Query validation for the lazy, allowlisted filesystem path picker. */ export const FilesystemBrowseQuerySchema = z.object({ path: filesystemPickerPathSchema.optional(), @@ -73,6 +81,7 @@ export const FilesystemBrowseQuerySchema = z.object({ .max(100) .regex(/^[a-zA-Z0-9_-]+$/, 'Invalid session id') .optional(), + showHidden: showHiddenQuerySchema, }); /** Query validation for a single allowlisted path-picker file preview. */ @@ -83,6 +92,7 @@ export const FilesystemPreviewQuerySchema = z.object({ .max(100) .regex(/^[a-zA-Z0-9_-]+$/, 'Invalid session id') .optional(), + showHidden: showHiddenQuerySchema, }); /** diff --git a/src/web/sensitive-path.ts b/src/web/sensitive-path.ts index cb3e8624..e50de370 100644 --- a/src/web/sensitive-path.ts +++ b/src/web/sensitive-path.ts @@ -13,23 +13,73 @@ * credentials, dotenv files) while leaving ordinary cross-workspace files * attachable. * + * ⚠️ The path picker's `showHidden` option is what makes the dot-prefixed half + * of this list load-bearing. Before it existed, the picker refused every path + * with a hidden segment, so `~/.config/gh/hosts.yml` and friends were + * unreachable by construction and the list only had to cover the few secrets + * that live in plain sight. Opting into hidden entries removes that accident, + * so every credential location below has to be named. Adding a new browse + * surface means re-reading this file, not assuming it already covers you. + * + * ⚠️ Deliberately NOT whole-tree blocks: `~/.codeman/` (the publish skill + * attaches from it) and `~/.claude/` (transcripts and team state are ordinary + * files worth attaching). Only their secret-bearing members are named. + * * Callers MUST resolve symlinks (realpath) BEFORE calling isSensitivePath so a * symlink pointing at a sensitive target is also caught. */ -import { homedir } from 'node:os'; - const SENSITIVE_PATTERNS: RegExp[] = [ + // System account databases. /^\/etc\/shadow$/, /^\/etc\/gshadow$/, /^\/etc\/master\.passwd$/, - new RegExp(`^${homedir().replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}\\/\\.ssh\\/`), + + // SSH and GPG private key material. `.ssh/` is matched at any depth rather + // than only under homedir(): a per-project or per-deploy key directory holds + // exactly the same secret, and it drops a homedir() read that is captured at + // module load and therefore wrong for anything that changes HOME later. + /\/\.ssh\//, + /\/\.gnupg\//, + + // Dotenv, in every conventional spelling (.env, .env.local, .env.production). /\/\.env$/, /\/\.env\./, - /\/credentials(\.json|\.yml|\.yaml|\.xml)?$/i, - /\/\.aws\/credentials$/, + + // Generic credential files, plus the per-vendor spellings that do not match it. + /\/credentials(\.json|\.yml|\.yaml|\.xml|\.toml|\.db)?$/i, + /\/\.aws\/(credentials|config)$/, + /\/\.aws\/sso\/cache\//, /\/\.gcloud\/credentials\.db$/, + /\/\.config\/gcloud\//, + /\/\.azure\//, /\/\.docker\/config\.json$/, + /\/\.kube\/config$/, + + // Package-registry and forge tokens. Each of these is a bearer credential in + // a plain-text dotfile, which is exactly what a path picker will surface. + /\/\.npmrc$/, + /\/\.yarnrc\.yml$/, + /\/\.git-credentials$/, + /\/\.config\/gh\//, + /\/\.config\/hub$/, + /\/\.netrc$/, + /\/_netrc$/, + /\/\.pypirc$/, + /\/\.gem\/credentials$/, + /\/\.cargo\/credentials(\.toml)?$/, + /\/\.terraformrc$/, + /\/\.terraform\.d\//, + + // Database client credentials. + /\/\.pgpass$/, + /\/\.my\.cnf$/, + + // Agent CLI credentials, including Codeman's own hook secret and user table. + // Named individually so the surrounding trees stay attachable (see above). + /\/\.claude\/\.credentials\.json$/, + /\/\.codeman[^/]*\/hook-secret$/, + /\/\.codeman[^/]*\/users\.json$/, ]; /** diff --git a/test/path-picker-hidden.test.ts b/test/path-picker-hidden.test.ts new file mode 100644 index 00000000..38aac4d1 --- /dev/null +++ b/test/path-picker-hidden.test.ts @@ -0,0 +1,187 @@ +/** + * @fileoverview PathPicker "show hidden" toggle (issue #221). + * + * `PathPicker` (keyboard-accessory.js) is the shared browser behind Link + * Existing's "Browse" and the mobile keyboard's `📁 Path` key, so one toggle + * serves both. What can silently go wrong here: + * + * 1. `showHidden` missing from the browse request (toggle looks dead), + * 2. `showHidden` missing from the PREVIEW request, which re-resolves the + * path independently, so the listing would show a hidden file that then + * 403s the moment you tap it, + * 3. the toggle resetting you to the root instead of reloading where you are, + * 4. the flag not surviving a reopen, or a `localStorage` throw taking the + * picker down with it. + * + * The picker builds its dialog with innerHTML and drives it through real + * listeners, so this needs a DOM rather than a `vm` stub. It runs in the DEFAULT + * node environment and constructs a jsdom window here, matching + * markdown-sanitizer.test.ts: a per-file jsdom environment directive + * externalizes node:fs under vite and the suite then fails to load. ⚠️ Do not + * write that directive's literal name anywhere in this file, not even in prose + * like this: vitest scans the whole source for it, so merely explaining the trap + * re-arms it. + */ +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import { JSDOM } from 'jsdom'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const PUBLIC = resolve(import.meta.dirname, '../src/web/public'); +const accessoryJs = readFileSync(resolve(PUBLIC, 'keyboard-accessory.js'), 'utf8'); +const stylesCss = readFileSync(resolve(PUBLIC, 'styles.css'), 'utf8'); + +const STORAGE_KEY = 'codeman:pathPickerShowHidden'; + +const dom = new JSDOM('', { url: 'https://localhost/' }); +const jsdomWindow = dom.window as unknown as Window & typeof globalThis; +const jsdomDocument = jsdomWindow.document; + +/** Evaluate keyboard-accessory.js against the jsdom window and return PathPicker. */ +function loadPathPicker(fetchImpl: (url: string) => Promise): any { + const MobileDetection = { isTouchDevice: () => false }; + const factory = new Function( + 'window', + 'document', + 'localStorage', + 'fetch', + 'MobileDetection', + `${accessoryJs}\nreturn PathPicker;` + ); + return factory(jsdomWindow, jsdomDocument, jsdomWindow.localStorage, fetchImpl, MobileDetection); +} + +function browseResponse(entries: Array<{ name: string; type: string }>, path = '/home/dev/project') { + return { + ok: true, + json: async () => ({ + success: true, + data: { + path, + parent: null, + root: '/home/dev', + roots: [{ label: 'Home', path: '/home/dev' }], + entries: entries.map((e) => ({ ...e, path: `${path}/${e.name}` })), + truncated: false, + }, + }), + }; +} + +describe('PathPicker show-hidden toggle', () => { + let PathPicker: any; + let urls: string[]; + let respond: (url: string) => unknown; + + beforeEach(() => { + jsdomWindow.localStorage.clear(); + jsdomDocument.body.replaceChildren(); + urls = []; + respond = () => + browseResponse([ + { name: '.github', type: 'directory' }, + { name: 'src', type: 'directory' }, + ]); + PathPicker = loadPathPicker(async (url: string) => { + urls.push(url); + return respond(url); + }); + }); + + afterEach(() => { + PathPicker?.close?.(false); + jsdomDocument.body.replaceChildren(); + }); + + const open = async (options: Record = {}) => { + PathPicker.open({ onSelect: () => {}, ...options }); + await vi.waitFor(() => expect(urls.length).toBeGreaterThan(0)); + }; + const toggle = () => jsdomDocument.querySelector('.path-picker-hidden') as HTMLButtonElement; + const previewHref = () => + (jsdomDocument.querySelector('.path-preview-open') as HTMLAnchorElement).getAttribute('href') ?? ''; + + it('omits showHidden by default', async () => { + await open(); + + expect(urls[0]).not.toContain('showHidden'); + expect(toggle().getAttribute('aria-pressed')).toBe('false'); + expect(toggle().classList.contains('active')).toBe(false); + expect(toggle().getAttribute('title')).toBe('Show hidden files and folders'); + }); + + it('sends showHidden=true after the toggle is pressed, and persists it', async () => { + await open(); + toggle().click(); + await vi.waitFor(() => expect(urls.length).toBe(2)); + + expect(urls[1]).toContain('showHidden=true'); + expect(jsdomWindow.localStorage.getItem(STORAGE_KEY)).toBe('1'); + expect(toggle().getAttribute('aria-pressed')).toBe('true'); + expect(toggle().classList.contains('active')).toBe(true); + expect(toggle().getAttribute('title')).toBe('Hide hidden files and folders'); + }); + + it('restores the preference when the picker is reopened', async () => { + jsdomWindow.localStorage.setItem(STORAGE_KEY, '1'); + await open(); + + expect(urls[0]).toContain('showHidden=true'); + expect(toggle().getAttribute('aria-pressed')).toBe('true'); + }); + + it('reloads the current folder rather than resetting to the root', async () => { + jsdomWindow.localStorage.setItem(STORAGE_KEY, '1'); + // Sitting inside a hidden folder, reachable only because the toggle is on. + respond = () => browseResponse([{ name: 'workflows', type: 'directory' }], '/home/dev/project/.github'); + await open({ initialPath: '/home/dev/project/.github' }); + + toggle().click(); + await vi.waitFor(() => expect(urls.length).toBe(2)); + + expect(decodeURIComponent(urls[1])).toContain('path=/home/dev/project/.github'); + expect(urls[1]).not.toContain('showHidden=true'); + }); + + it('carries the flag into the preview request', async () => { + jsdomWindow.localStorage.setItem(STORAGE_KEY, '1'); + await open(); + + PathPicker.openPreview({ name: '.gitignore', path: '/home/dev/project/.gitignore', previewKind: 'text' }); + + expect(previewHref()).toContain('showHidden=true'); + }); + + it('leaves the preview flag off when the toggle is off', async () => { + await open(); + + PathPicker.openPreview({ name: 'notes.txt', path: '/home/dev/project/notes.txt', previewKind: 'text' }); + + expect(previewHref()).not.toContain('showHidden'); + }); + + it('survives a localStorage that throws (private browsing)', async () => { + const storage = Object.getPrototypeOf(jsdomWindow.localStorage); + const getItem = vi.spyOn(storage, 'getItem').mockImplementation(() => { + throw new Error('denied'); + }); + const setItem = vi.spyOn(storage, 'setItem').mockImplementation(() => { + throw new Error('denied'); + }); + try { + await open(); + expect(urls[0]).not.toContain('showHidden'); + + toggle().click(); + await vi.waitFor(() => expect(urls.length).toBe(2)); + expect(urls[1]).toContain('showHidden=true'); + } finally { + getItem.mockRestore(); + setItem.mockRestore(); + } + }); + + it('styles the active toggle so it reads as on', () => { + expect(stylesCss).toContain('.path-picker-hidden.active'); + }); +}); diff --git a/test/routes/file-routes.test.ts b/test/routes/file-routes.test.ts index 8f92cebb..ee975a47 100644 --- a/test/routes/file-routes.test.ts +++ b/test/routes/file-routes.test.ts @@ -167,6 +167,118 @@ describe('file-routes', () => { expect(res.statusCode).toBe(403); expect(JSON.parse(res.body)).toMatchObject({ success: false, errorCode: ApiErrorCode.INVALID_INPUT }); }); + + // ===== showHidden=true (issue #221) ===== + // + // The dotfile filter used to be doing security work by accident: with every + // hidden path unreachable, the sensitive-path blocklist never had to cover + // `~/.config/gh/hosts.yml` and friends. These pin that opting in lifts the + // hidden filter and NOTHING else — blocked trees, sensitive files and root + // confinement all still apply. + describe('showHidden=true', () => { + it('lists dot-prefixed entries', async () => { + mockedReaddir.mockResolvedValueOnce([ + { name: '.github', isDirectory: () => true, isFile: () => false, isSymbolicLink: () => false }, + { name: '.gitignore', isDirectory: () => false, isFile: () => true, isSymbolicLink: () => false }, + { name: 'src', isDirectory: () => true, isFile: () => false, isSymbolicLink: () => false }, + ] as never); + + const root = harness.ctx._session.workingDir; + const res = await harness.app.inject({ + method: 'GET', + url: `/api/filesystem/browse?sessionId=${harness.ctx._sessionId}&path=${encodeURIComponent(root)}&showHidden=true`, + }); + + expect(res.statusCode).toBe(200); + expect(JSON.parse(res.body).data.entries.map((e: { name: string }) => e.name)).toEqual([ + '.github', + 'src', + '.gitignore', + ]); + }); + + it('allows navigating into a hidden descendant', async () => { + mockedReaddir.mockResolvedValueOnce([ + { name: 'workflows', isDirectory: () => true, isFile: () => false, isSymbolicLink: () => false }, + ] as never); + + const hidden = `${harness.ctx._session.workingDir}/.github`; + const res = await harness.app.inject({ + method: 'GET', + url: `/api/filesystem/browse?sessionId=${harness.ctx._sessionId}&path=${encodeURIComponent(hidden)}&showHidden=true`, + }); + + expect(res.statusCode).toBe(200); + expect(JSON.parse(res.body).data.path).toBe(hidden); + }); + + it('still hides dot-prefixed entries when the flag is absent or false', async () => { + const entries = [ + { name: '.gitignore', isDirectory: () => false, isFile: () => true, isSymbolicLink: () => false }, + { name: 'src', isDirectory: () => true, isFile: () => false, isSymbolicLink: () => false }, + ]; + const root = harness.ctx._session.workingDir; + + for (const query of ['', '&showHidden=false']) { + mockedReaddir.mockResolvedValueOnce(entries as never); + const res = await harness.app.inject({ + method: 'GET', + url: `/api/filesystem/browse?sessionId=${harness.ctx._sessionId}&path=${encodeURIComponent(root)}${query}`, + }); + expect(res.statusCode).toBe(200); + expect(JSON.parse(res.body).data.entries.map((e: { name: string }) => e.name)).toEqual(['src']); + } + }); + + it('rejects a showHidden value that is not a boolean string', async () => { + const res = await harness.app.inject({ + method: 'GET', + url: `/api/filesystem/browse?sessionId=${harness.ctx._sessionId}&showHidden=yes`, + }); + + expect(res.statusCode).toBe(400); + expect(JSON.parse(res.body)).toMatchObject({ success: false, errorCode: ApiErrorCode.INVALID_INPUT }); + }); + + it('still omits blocked and sensitive entries', async () => { + const root = harness.ctx._session.workingDir; + mockedReaddir.mockResolvedValueOnce([ + { name: '.ssh', isDirectory: () => true, isFile: () => false, isSymbolicLink: () => false }, + { name: '.npmrc', isDirectory: () => false, isFile: () => true, isSymbolicLink: () => false }, + { name: '.env', isDirectory: () => false, isFile: () => true, isSymbolicLink: () => false }, + { name: '.gitignore', isDirectory: () => false, isFile: () => true, isSymbolicLink: () => false }, + // A plainly-named symlink whose target is a secret: caught on the + // resolved path, not the visible name. + { name: 'notes', isDirectory: () => false, isFile: () => false, isSymbolicLink: () => true }, + ] as never); + mockedRealpathSync.mockImplementation((p: string) => + p === `${root}/notes` ? (`${root}/.aws/credentials` as never) : (p as never) + ); + + const res = await harness.app.inject({ + method: 'GET', + url: `/api/filesystem/browse?sessionId=${harness.ctx._sessionId}&path=${encodeURIComponent(root)}&showHidden=true`, + }); + + expect(res.statusCode).toBe(200); + expect(JSON.parse(res.body).data.entries.map((e: { name: string }) => e.name)).toEqual(['.gitignore']); + }); + + it('refuses a hidden path that resolves outside every root', async () => { + const outside = `${harness.ctx._session.workingDir}/.cache`; + mockedRealpathSync.mockImplementation((p: string) => + p === outside ? ('/tmp/somewhere-else' as never) : (p as never) + ); + + const res = await harness.app.inject({ + method: 'GET', + url: `/api/filesystem/browse?sessionId=${harness.ctx._sessionId}&path=${encodeURIComponent(outside)}&showHidden=true`, + }); + + expect(res.statusCode).toBe(403); + expect(JSON.parse(res.body)).toMatchObject({ success: false, errorCode: ApiErrorCode.INVALID_INPUT }); + }); + }); }); // ========== Multi-user scoping for the filesystem picker ========== diff --git a/test/sensitive-path.test.ts b/test/sensitive-path.test.ts new file mode 100644 index 00000000..264d9966 --- /dev/null +++ b/test/sensitive-path.test.ts @@ -0,0 +1,110 @@ +/** + * @fileoverview The shared sensitive-path blocklist (`src/web/sensitive-path.ts`). + * + * This list guards every browser-facing file surface: workspace download, + * cross-workspace attachment registration, raw/preview serving, and the + * filesystem path picker. + * + * It became load-bearing when the picker gained `showHidden` (issue #221). + * Before that, the picker refused any path with a dot-prefixed segment, so most + * of the credential locations below were unreachable by construction and the + * list only had to cover secrets that sit in plain sight. Opting into hidden + * entries removes that accident, which is why each entry is pinned here: a + * pattern silently dropped in a refactor would re-expose a real token. + * + * The list is a BLOCKLIST by design (cross-workspace attachment is a supported + * feature), so the "stays attachable" cases matter just as much: over-blocking + * breaks the publish skill and the review-card loop. + */ +import { describe, expect, it } from 'vitest'; +import { isSensitivePath } from '../src/web/sensitive-path.js'; + +const HOME = '/home/dev'; + +describe('isSensitivePath', () => { + describe('blocks', () => { + const blocked: Array<[string, string]> = [ + ['system shadow file', '/etc/shadow'], + ['system gshadow file', '/etc/gshadow'], + ['BSD master password db', '/etc/master.passwd'], + + ['ssh keys in home', `${HOME}/.ssh/id_ed25519`], + // Not only under homedir(): a deploy key in a project is the same secret, + // and the old homedir()-anchored pattern was captured at module load. + ['ssh keys anywhere', '/srv/deploy/.ssh/id_rsa'], + ['gpg keyring', `${HOME}/.gnupg/private-keys-v1.d/key.key`], + + ['dotenv', '/srv/app/.env'], + ['suffixed dotenv', '/srv/app/.env.production'], + // Pre-existing and deliberate: `.env.*` is blocked wholesale, so even a + // committed `.env.example` is refused rather than risking the one repo + // whose "example" holds a live key. + ['a dotenv example', '/srv/app/.env.example'], + + ['generic credentials file', '/srv/app/credentials'], + ['json credentials', '/srv/app/credentials.json'], + ['toml credentials', '/srv/app/credentials.toml'], + ['aws credentials', `${HOME}/.aws/credentials`], + ['aws config', `${HOME}/.aws/config`], + ['aws sso cache', `${HOME}/.aws/sso/cache/abc.json`], + ['legacy gcloud credential db', `${HOME}/.gcloud/credentials.db`], + ['modern gcloud config tree', `${HOME}/.config/gcloud/application_default_credentials.json`], + ['azure profile', `${HOME}/.azure/accessTokens.json`], + ['docker registry auth', `${HOME}/.docker/config.json`], + ['kubernetes context', `${HOME}/.kube/config`], + + ['npm token', `${HOME}/.npmrc`], + ['yarn token', `${HOME}/.yarnrc.yml`], + ['git credential store', `${HOME}/.git-credentials`], + ['gh cli token', `${HOME}/.config/gh/hosts.yml`], + ['hub token', `${HOME}/.config/hub`], + ['netrc', `${HOME}/.netrc`], + ['windows netrc', `${HOME}/_netrc`], + ['pypi token', `${HOME}/.pypirc`], + ['rubygems token', `${HOME}/.gem/credentials`], + ['cargo token', `${HOME}/.cargo/credentials.toml`], + ['terraform cli config', `${HOME}/.terraformrc`], + ['terraform credentials dir', `${HOME}/.terraform.d/credentials.tfrc.json`], + + ['postgres password file', `${HOME}/.pgpass`], + ['mysql client config', `${HOME}/.my.cnf`], + + ['claude oauth token', `${HOME}/.claude/.credentials.json`], + ['codeman hook secret', `${HOME}/.codeman/hook-secret`], + ['codeman user table', `${HOME}/.codeman/users.json`], + ['codeman hook secret on a named instance', `${HOME}/.codeman-beta/hook-secret`], + ]; + + it.each(blocked)('blocks the %s', (_label, path) => { + expect(isSensitivePath(path)).toBe(true); + }); + }); + + describe('leaves ordinary files attachable', () => { + const allowed: Array<[string, string]> = [ + ['a source file', '/srv/app/src/index.ts'], + ['a dotfile that carries no secret', '/srv/app/.gitignore'], + ['a hidden CI directory', '/srv/app/.github/workflows/ci.yml'], + // The publish skill and the review-card loop attach from these trees, so + // only their named secret members are blocked, never the whole tree. + ['a codeman screenshot', `${HOME}/.codeman/screenshots/shot.png`], + ['a claude transcript', `${HOME}/.claude/projects/proj/session.jsonl`], + ['a claude team inbox', `${HOME}/.claude/teams/alpha/inboxes/bob.json`], + // isUnderTree-style separator awareness: a sibling name that merely starts + // with a blocked segment must not be caught. + ['an unrelated sshd notes file', '/srv/notes/.sshd-setup.md'], + ['a file named credentials-policy.md', '/srv/app/credentials-policy.md'], + ]; + + it.each(allowed)('allows %s', (_label, path) => { + expect(isSensitivePath(path)).toBe(false); + }); + }); + + it('matches on the resolved path, so callers must realpath first', () => { + // The function itself is pure string matching; this pins the contract its + // docblock states, which every caller depends on. + expect(isSensitivePath('/srv/app/looks-innocent')).toBe(false); + expect(isSensitivePath(`${HOME}/.ssh/looks-innocent`)).toBe(true); + }); +});