mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
Merge pull request #250 from Ark0N/feat/path-picker-show-hidden
feat(path-picker): show hidden files and folders, and harden the secret blocklist
This commit is contained in:
Ark0N
@@ -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