Merge pull request #310 from Ark0N/fix/files-sidebar-followups

fix: file-link and session-sidebar review follow-ups from 1.19.0
This commit is contained in:
Ark0N
2026-08-16 20:44:14 +02:00
committed by GitHub
9 changed files with 209 additions and 13 deletions
+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', () => {