fix: file-link and session-sidebar review follow-ups from 1.19.0

Five post-merge review items from PRs #306 (clickable file paths) and
#307 (session sidebar):

- constants.js FILE_PREVIEW_EXTENSIONS gains the media extensions it was
  missing vs the single-source sets in attachment-registry.ts (m4v ogv
  ogg oga m4a aac flac opus), so an in-workspace .m4a opens the preview
  player instead of the log viewer; new test/media-extension-parity.test.ts
  pins all three copies (constants.js, panels-ui.js, attachment-registry.ts)
  against each other.
- FILE_PATH_LINK_PATTERN drops `etc` from its root alternation: /etc is
  unconditionally in DEFAULT_BLOCKED_TREES, so every /etc link 403'd.
  Negative cases added to the link-provider and response-viewer tests.
- updateSidebarCount() counts the rows actually on the sidebar list
  (session rows + web-tab rows, minus filtered-out ones) instead of
  this.sessions.size, and applySidebarFilter() refreshes it so the count
  follows the filter box per keystroke.
- The incremental-render connection-line gate now also fires in sidebar
  layout (this._lineageEdgeCount is permanently 0 there), matching the
  strip-scroll listener widened in #307, so a badge changing row heights
  redraws subagent/ultracode connectors.
- isSensitivePath() blocks ~/.claude.json, ~/.claude/settings.json and
  ~/.claude/settings.local.json (credential-bearing by schema), anchored
  to homedir() read at check time so case-level .claude/settings*.json
  files stay servable in the File Viewer.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer
2026-08-16 20:34:34 +02:00
parent f07905b193
commit 24ed43935c
9 changed files with 209 additions and 13 deletions
+22 -6
View File
@@ -3826,7 +3826,6 @@ class CodemanApp {
// Collapse/expand changes whether the filter is reachable, so re-evaluate it
// here too — not only at the render tails.
this.applySidebarFilter(this._sidebarFilter);
this.updateSidebarCount();
this.updateConnectionLines();
// The desktop home rail defers to the sidebar (both dock the session list
// flush left), so a layout flip while the welcome screen is up has to
@@ -3871,9 +3870,22 @@ class CodemanApp {
this.toggleSessionSidebar();
}
/**
* The count is what is actually ON the list: session rows plus web-tab rows,
* minus whatever the sidebar filter is hiding. `this.sessions.size` was the
* original source and disagreed with the screen twice over — web tabs render
* in the same list but are not sessions (3 sessions + 2 dashboards read "3"
* above 5 rows), and a filter hides rows without touching the map. Counting
* the rendered rows keeps one source of truth: the list itself.
*/
updateSidebarCount() {
const el = document.getElementById('sessionSidebarCount');
if (el) el.textContent = String(this.sessions?.size ?? 0);
if (!el) return;
const container = this.$('sessionTabs');
const count = container
? container.querySelectorAll('.session-tab:not(.tab-filtered-out)').length
: (this.sessions?.size ?? 0);
el.textContent = String(count);
}
/**
@@ -3906,6 +3918,9 @@ class CodemanApp {
const haystack = `${tab.getAttribute('aria-label') || ''} ${tab.getAttribute('title') || ''}`.toLowerCase();
tab.classList.toggle('tab-filtered-out', !haystack.includes(needle));
}
// The count shows visible rows, so it moves with every filter change —
// including keystrokes in the filter box, which call this directly.
this.updateSidebarCount();
}
// ═══════════════════════════════════════════════════════════════
@@ -4262,11 +4277,13 @@ class CodemanApp {
// The full-render path already redraws the connection SVG; this incremental
// one does not, and a badge appearing widens a tab and shifts every tab after
// it, sliding the lineage arcs off their anchors. Only pay for it when there
// is an arc to keep anchored.
if (this._lineageEdgeCount > 0) this.updateConnectionLines();
// is something anchored to tab rects: lineage arcs, or — in sidebar layout,
// where lineage is skipped and the edge count stays 0 — the subagent/
// ultracode connectors, whose rows a badge changes the HEIGHT of. Same
// widening as the strip-scroll listener in session-lineage.js.
if (this._lineageEdgeCount > 0 || this.isSessionSidebarActive()) this.updateConnectionLines();
this.applySidebarFilter(this._sidebarFilter);
this.updateSidebarCount();
}
// Auto-wrap desktop session tabs to a second row when they overflow one row,
@@ -4467,7 +4484,6 @@ class CodemanApp {
// innerHTML was rebuilt wholesale, so the sidebar filter classes are gone —
// re-apply them or filtered-out sessions flicker back on every SSE tick.
this.applySidebarFilter(this._sidebarFilter);
this.updateSidebarCount();
}
// Set up arrow key navigation for session tabs (accessibility)
+11 -3
View File
@@ -997,13 +997,16 @@ function computeRewriteScrollLine(input) {
* never match) and terminated by a known extension (so the end of the path is
* unambiguous — a trailing `)` or `.` after the extension stays out). Longer
* extensions come first in each family (`tsx|ts`), so the trailing `\b` cannot
* be satisfied by the shorter branch mid-word.
* be satisfied by the shorter branch mid-word. `/etc` is deliberately NOT a
* root: DEFAULT_BLOCKED_TREES (config/attachment-guard.ts) refuses the whole
* tree server-side, so every `/etc/...` link was a guaranteed 403 — a link
* that renders clickable and then dies is worse than plain text.
*
* ⚠ Consumers must never share one instance: `lastIndex` is per-object state on
* a `/g` regex, so {@link absoluteFilePathPattern} mints a fresh one per call.
*/
const FILE_PATH_LINK_PATTERN =
/(\/(?:home|Users|tmp|var|private|etc|opt|mnt|srv|media|data|workspace)\/[^\s"'<>|;&\n\x00-\x1f]*\.(?:log|txt|json|md|ya?ml|csv|xml|sh|py|tsx|ts|jsx|js|mjs|cjs|css|html|toml|ini|sql|png|jpe?g|gif|webp|bmp|svg|pdf|docx|pptx|mp4|webm|mov|mp3|wav))\b/g;
/(\/(?:home|Users|tmp|var|private|opt|mnt|srv|media|data|workspace)\/[^\s"'<>|;&\n\x00-\x1f]*\.(?:log|txt|json|md|ya?ml|csv|xml|sh|py|tsx|ts|jsx|js|mjs|cjs|css|html|toml|ini|sql|png|jpe?g|gif|webp|bmp|svg|pdf|docx|pptx|mp4|webm|mov|mp3|wav))\b/g;
/** A fresh, zero-state instance of {@link FILE_PATH_LINK_PATTERN}. */
function absoluteFilePathPattern() {
@@ -1014,9 +1017,14 @@ function absoluteFilePathPattern() {
* Extensions the file-preview overlay renders itself. Everything else a link
* points at goes to the tail/log viewer, which is the right home for a growing
* text file and the wrong one for bytes (tailing a PNG shows binary noise).
*
* The media entries mirror VIDEO_ATTACHMENT_EXTENSIONS/AUDIO_ATTACHMENT_EXTENSIONS
* (src/attachment-registry.ts, the single source) — they diverged once and an
* in-workspace `.m4a` opened as binary noise in the log viewer while the same
* file in /tmp played fine. test/media-extension-parity.test.ts pins the sync.
*/
const FILE_PREVIEW_EXTENSIONS = new Set(
('png jpg jpeg gif webp bmp svg pdf docx pptx mp4 webm mov mp3 wav').split(' ')
('png jpg jpeg gif webp bmp svg pdf docx pptx mp4 webm mov m4v ogv mp3 wav ogg oga m4a aac flac opus').split(' ')
);
/** Whether a path's extension is one {@link FILE_PREVIEW_EXTENSIONS} covers. */
+3
View File
@@ -3346,6 +3346,9 @@ Object.assign(CodemanApp.prototype, {
if (attachmentId) {
const base = `/api/sessions/${sessionId}/attachments/${encodeURIComponent(attachmentId)}`;
const IMAGE_EXTS = new Set(['png', 'jpg', 'jpeg', 'gif', 'webp', 'bmp', 'svg']);
// VIDEO/AUDIO mirror VIDEO_ATTACHMENT_EXTENSIONS/AUDIO_ATTACHMENT_EXTENSIONS
// (src/attachment-registry.ts, the single source); the frontend cannot import
// it, so test/media-extension-parity.test.ts pins the copies equal.
const VIDEO_EXTS = new Set(['mp4', 'webm', 'mov', 'm4v', 'ogv']);
const AUDIO_EXTS = new Set(['mp3', 'wav', 'ogg', 'oga', 'm4a', 'aac', 'flac', 'opus']);
// Size when we just registered the file ourselves, so a path opened from a
+20 -1
View File
@@ -29,6 +29,9 @@
* symlink pointing at a sensitive target is also caught.
*/
import { homedir } from 'node:os';
import { join } from 'node:path';
const SENSITIVE_PATTERNS: RegExp[] = [
// System account databases.
/^\/etc\/shadow$/,
@@ -99,10 +102,26 @@ const SENSITIVE_PATTERNS: RegExp[] = [
/\/\.codeman[^/]*\/intents\.json$/,
];
/**
* Claude config members that are credential-bearing ONLY under the user's real
* home directory: `~/.claude/settings.json` can hold `env.ANTHROPIC_API_KEY`
* and `apiKeyHelper` by schema (settings.local.json shares that schema), and
* `~/.claude.json` holds account/OAuth-adjacent state. A blanket
* `/\.claude\/settings\.json$/` would also block every CASE-level
* `.claude/settings.json`, which users legitimately view and edit in the File
* Viewer (model override, hooks) — so these are anchored to homedir(), read at
* CHECK time inside isSensitivePath, never captured at module load (wrong for
* anything that changes HOME later, e.g. per-file test fixtures — same
* reasoning as the `.ssh/` note above).
*/
const HOME_SENSITIVE_MEMBERS = ['.claude.json', '.claude/settings.json', '.claude/settings.local.json'];
/**
* Returns true if the given ABSOLUTE, symlink-resolved path matches the
* sensitive-file blocklist and must not be served to the browser.
*/
export function isSensitivePath(absPath: string): boolean {
return SENSITIVE_PATTERNS.some((pattern) => pattern.test(absPath));
if (SENSITIVE_PATTERNS.some((pattern) => pattern.test(absPath))) return true;
const home = homedir();
return HOME_SENSITIVE_MEMBERS.some((member) => absPath === join(home, member));
}
+17
View File
@@ -150,6 +150,23 @@ describe('terminal link-provider regexes (shipped source)', () => {
}
});
it('the file-path pattern refuses /etc roots (blocked server-side, so the link could only 403)', () => {
// `/etc` sits in DEFAULT_BLOCKED_TREES (config/attachment-guard.ts), so an
// /etc link is guaranteed dead: it renders clickable, then the preview 403s.
// It used to be in the root alternation, which linked exactly those paths.
const ext = shippedPattern('FILE_PATH_LINK_PATTERN');
const cases = [
'see /etc/hosts here',
// Extension-bearing, so only the root removal keeps it out.
'see /etc/app/config.json here',
'cat /etc/nginx/nginx.conf.txt',
];
for (const line of cases) {
ext.lastIndex = 0;
expect(ext.exec(line), line).toBeNull();
}
});
it('terminal-ui builds its path pattern from the shared factory', () => {
// Structural guard: a local literal here would drift from the response
// viewer's linkifier, which is the divergence the move exists to prevent.
+75
View File
@@ -0,0 +1,75 @@
/**
* @fileoverview Media-extension parity — attachment registry ⇄ frontend copies.
*
* CLAUDE.md single-sources playable media extensions in
* `VIDEO_ATTACHMENT_EXTENSIONS`/`AUDIO_ATTACHMENT_EXTENSIONS`
* (src/attachment-registry.ts): the workspace preview and the out-of-workspace
* attachment path must agree on what plays. The frontend cannot import that
* module, so two hand-maintained copies exist and BOTH have drifted:
*
* - `FILE_PREVIEW_EXTENSIONS` (constants.js) decides whether a clicked
* terminal/chat path opens the preview overlay or the tail/log viewer. It
* was missing `m4v ogv ogg oga m4a aac flac opus`, so an in-workspace
* `.m4a` routed to the log viewer and rendered as binary noise while the
* same file in /tmp played fine.
* - `VIDEO_EXTS`/`AUDIO_EXTS` (panels-ui.js) pick the <video>/<audio> markup
* for registered attachments; an entry missing there renders a text dump
* instead of a player.
*
* Same technique as test/sse-registry-parity.test.ts: the backend sets are
* imported, the frontend copies are extracted from the shipped source as text
* (no build-time link exists), and the sets are compared. No port needed.
*/
import { describe, expect, it } from 'vitest';
import { readFileSync } from 'node:fs';
import { resolve } from 'node:path';
import { AUDIO_ATTACHMENT_EXTENSIONS, VIDEO_ATTACHMENT_EXTENSIONS } from '../src/attachment-registry.js';
const publicFile = (name: string) =>
readFileSync(resolve(import.meta.dirname, '..', 'src', 'web', 'public', name), 'utf8');
/** `FILE_PREVIEW_EXTENSIONS` is a space-separated string literal in constants.js. */
function filePreviewExtensions(): Set<string> {
const src = publicFile('constants.js');
const m = src.match(/const FILE_PREVIEW_EXTENSIONS = new Set\(\s*\('([^']+)'\)\.split\(' '\)\s*\)/);
expect(m, 'FILE_PREVIEW_EXTENSIONS literal not found in constants.js').not.toBeNull();
return new Set(m![1].split(' '));
}
/** `VIDEO_EXTS`/`AUDIO_EXTS` are quoted-string array Sets in panels-ui.js. */
function panelsUiSet(name: string): Set<string> {
const src = publicFile('panels-ui.js');
const m = src.match(new RegExp(`const ${name} = new Set\\(\\[([^\\]]+)\\]\\)`));
expect(m, `${name} literal not found in panels-ui.js`).not.toBeNull();
const values = [...m![1].matchAll(/'([^']+)'/g)].map((q) => q[1]);
return new Set(values);
}
const sorted = (s: ReadonlySet<string>) => [...s].sort();
describe('media extension parity (attachment registry ⇄ frontend)', () => {
it('extracts non-trivial sets from every source (guards the parsers)', () => {
expect(VIDEO_ATTACHMENT_EXTENSIONS.size).toBeGreaterThanOrEqual(5);
expect(AUDIO_ATTACHMENT_EXTENSIONS.size).toBeGreaterThanOrEqual(8);
expect(filePreviewExtensions().size).toBeGreaterThan(10);
expect(panelsUiSet('VIDEO_EXTS').size).toBeGreaterThanOrEqual(5);
expect(panelsUiSet('AUDIO_EXTS').size).toBeGreaterThanOrEqual(8);
});
it('every playable media extension routes to the preview overlay, not the log viewer', () => {
const preview = filePreviewExtensions();
const missing = [...VIDEO_ATTACHMENT_EXTENSIONS, ...AUDIO_ATTACHMENT_EXTENSIONS].filter((e) => !preview.has(e));
expect(
missing,
`media extensions in attachment-registry.ts but not constants.js FILE_PREVIEW_EXTENSIONS: ${missing.join(', ')}`
).toEqual([]);
});
it("panels-ui.js VIDEO_EXTS exactly equals the registry's video set", () => {
expect(sorted(panelsUiSet('VIDEO_EXTS'))).toEqual(sorted(VIDEO_ATTACHMENT_EXTENSIONS));
});
it("panels-ui.js AUDIO_EXTS exactly equals the registry's audio set", () => {
expect(sorted(panelsUiSet('AUDIO_EXTS'))).toEqual(sorted(AUDIO_ATTACHMENT_EXTENSIONS));
});
});
+10
View File
@@ -117,6 +117,16 @@ describe('response viewer file-path linkifier', () => {
expect(root.textContent).toBe('Ratio 3/4 on 2026/08/16, see src/app.ts');
});
it('never linkifies /etc paths — the server blocks the whole tree, so the link could only 403', () => {
// /etc sits in DEFAULT_BLOCKED_TREES (config/attachment-guard.ts); it used
// to be a root in the shared pattern, which made every /etc link a
// guaranteed-dead click on both surfaces.
const root = linkify('<p>Check /etc/hosts and /etc/app/config.json for the mapping.</p>');
expect(paths(root)).toHaveLength(0);
expect(root.textContent).toBe('Check /etc/hosts and /etc/app/config.json for the mapping.');
});
it('cannot turn model text into markup', () => {
// The anchor is built with createElement + textContent, so even a
// path-shaped payload stays text. (`<` also ends a match, so the linkifier
+31
View File
@@ -16,6 +16,7 @@
* feature), so the "stays attachable" cases matter just as much: over-blocking
* breaks the publish skill and the review-card loop.
*/
import { homedir } from 'node:os';
import { describe, expect, it } from 'vitest';
import { isSensitivePath } from '../src/web/sensitive-path.js';
@@ -122,4 +123,34 @@ describe('isSensitivePath', () => {
expect(isSensitivePath('/srv/app/looks-innocent')).toBe(false);
expect(isSensitivePath(`${HOME}/.ssh/looks-innocent`)).toBe(true);
});
describe('home-anchored Claude config (credential-bearing by schema)', () => {
// ~/.claude/settings.json can hold `env: {ANTHROPIC_API_KEY}` and
// `apiKeyHelper` by schema (settings.local.json shares it), and
// ~/.claude.json holds account/OAuth-adjacent state. These are anchored to
// the REAL homedir, read at CHECK time — test/setup.ts points HOME at a
// per-file fixture, so a homedir() captured at module load would be a
// different directory than the one this suite resolves.
const home = homedir();
it.each([
['claude account state', `${home}/.claude.json`],
['claude user settings', `${home}/.claude/settings.json`],
['claude user local settings', `${home}/.claude/settings.local.json`],
])('blocks the %s', (_label, path) => {
expect(isSensitivePath(path)).toBe(true);
});
// A blanket `/\.claude\/settings\.json$/` would also catch every CASE-level
// settings file, which users legitimately view and edit in the File Viewer
// (model override, hooks) — the home anchor is what keeps those servable.
it.each([
['a case-level .claude/settings.json', '/srv/app/.claude/settings.json'],
['a case-level .claude/settings.local.json', '/srv/app/.claude/settings.local.json'],
['a .claude/settings.json under some OTHER home', `${HOME}/.claude/settings.json`],
['a .claude.json under some OTHER home', `${HOME}/.claude.json`],
])('keeps %s servable', (_label, path) => {
expect(isSensitivePath(path)).toBe(false);
});
});
});
+20 -3
View File
@@ -394,15 +394,32 @@ describe('session list layout', () => {
expect((drawer.win.document.activeElement as HTMLElement).className).toContain('session-tab');
});
it('shows the live session count in the sidebar header', () => {
it('counts the rows actually on the list: web tabs included, filtered rows excluded', () => {
// this.sessions.size was the original source and disagreed with the screen
// twice over: web tabs render in the same list but are not sessions (3
// sessions + 2 dashboards read "3" above 5 rows), and the filter hides
// rows without touching the map.
const { win, app } = boot({ stored: { sessionListLayout: 'sidebar' } });
app.sessions = new Map([
['a', {}],
['b', {}],
['c', {}],
]);
app.applySessionListLayout();
expect(win.document.getElementById('sessionSidebarCount')?.textContent).toBe('3');
tabsEl(win).innerHTML = `
<div class="session-tab" data-id="a" aria-label="api server" title="/srv/api"></div>
<div class="session-tab" data-id="b" aria-label="docs" title="/home/docs"></div>
<div class="session-tab session-tab--web" data-webview-id="w" aria-label="Grafana web tab" title="http://x/g"></div>
`;
app.updateSidebarCount();
const count = () => win.document.getElementById('sessionSidebarCount')?.textContent;
expect(count()).toBe('3');
// The count follows the filter — applySidebarFilter is what the filter box
// calls per keystroke, so it must move without waiting for a re-render.
app.applySidebarFilter('api');
expect(count()).toBe('1');
app.applySidebarFilter('');
expect(count()).toBe('3');
});
it('forces tall rows and no wrapping in the sidebar, and leaves the strip rules alone', () => {