mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-05 06:59:42 +02:00
feat(path-picker): show hidden files and folders, and harden the secret blocklist
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 be opened at all. It gains the same `.*` toggle as the File Viewer: default OFF, per-device, and applied to both the listing and the preview endpoint, which re-resolves the path independently. That dotfile filter was quietly doing security work. The picker's roots include Home, so with every hidden path unreachable the shared blocklist never had to name the credentials that live in dot-directories. Lifting the filter removes that accident, so `isSensitivePath` now covers them explicitly: SSH keys at any depth rather than 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. Everything else still applies with the toggle on: blocked trees, sensitive files, root confinement, ownership scoping and symlink-escape checks. A hidden entry whose realpath is a secret is dropped from the listing, and opening it is refused. Follows #221 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer
@@ -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('<!DOCTYPE html><html><body></body></html>', { 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<unknown>): 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<string, unknown> = {}) => {
|
||||
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');
|
||||
});
|
||||
});
|
||||
@@ -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 ==========
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user