mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-01 19:25:10 +02:00
fix: community security wave — 8 PRs, 4 contributors (v0.15.13.0) (#847)
* fix(bin): pass search params via env vars (RCE fix) (#819) Replace shell string interpolation with process.env in gstack-learnings-search to prevent arbitrary code execution via crafted learnings entries. Also fixes the CROSS_PROJECT interpolation that the original PR missed. Adds 3 regression tests verifying no shell interpolation remains in the bun -e block. Co-authored-by: garagon <garagon@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(browse): add path validation to upload command (#821) Add isPathWithin() and path traversal checks to the upload command, blocking file exfiltration via crafted upload paths. Uses existing SAFE_DIRECTORIES constant instead of a local copy. Adds 3 regression tests. Co-authored-by: garagon <garagon@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(browse): symlink resolution in meta-commands validateOutputPath (#820) Add realpathSync to validateOutputPath in meta-commands.ts to catch symlink-based directory escapes in screenshot, pdf, and responsive commands. Resolves SAFE_DIRECTORIES through realpathSync to handle macOS /tmp -> /private/tmp symlinks. Existing path validation tests pass with the hardened implementation. Co-authored-by: garagon <garagon@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: add uninstall instructions to README (#812) Community PR #812 by @0531Kim. Adds two uninstall paths: the gstack-uninstall script (handles everything) and manual removal steps for when the repo isn't cloned. Includes CLAUDE.md cleanup note and Playwright cache guidance. Co-Authored-By: 0531Kim <0531Kim@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(browse): Windows launcher extraEnv + headed-mode token (#822) Community PR #822 by @pieterklue. Three fixes: 1. Windows launcher now merges extraEnv into spawned server env (was only passing BROWSE_STATE_FILE, dropping all other env vars) 2. Welcome page fallback serves inline HTML instead of about:blank redirect (avoids ERR_UNSAFE_REDIRECT on Windows) 3. /health returns auth token in headed mode even without Origin header (fixes Playwright Chromium extensions that don't send it) Also adds HOME/USERPROFILE fallback for cross-platform compatibility. Co-Authored-By: pieterklue <pieterklue@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(browse): terminate orphan server when parent process exits (#808) Community PR #808 by @mmporong. Passes BROWSE_PARENT_PID to the spawned server process. The server polls every 15s with signal 0 and calls shutdown() if the parent is gone. Prevents orphaned chrome-headless-shell processes when Claude Code sessions exit abnormally. Co-Authored-By: mmporong <mmporong@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(security): IPv6 ULA blocking, cookie redaction, per-tab cancel, targeted token (#664) Community PR #664 by @mr-k-man (security audit round 1, new parts only). - IPv6 ULA prefix blocking (fc00::/7) in url-validation.ts with false-positive guard for hostnames like fd.example.com - Cookie value redaction for tokens, API keys, JWTs in browse cookies command - Per-tab cancel files in killAgent() replacing broken global kill-signal - design/serve.ts: realpathSync upgrade prevents symlink bypass in /api/reload - extension: targeted getToken handler replaces token-in-health-broadcast - Supabase migration 003: column-level GRANT restricts anon UPDATE scope - Telemetry sync: upsert error logging - 10 new tests for IPv6, cookie redaction, DNS rebinding, path traversal Co-Authored-By: mr-k-man <mr-k-man@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(security): CSS injection guard, timeout clamping, session validation, tests (#806) Community PR #806 by @mr-k-man (security audit round 2, new parts only). - CSS value validation (DANGEROUS_CSS) in cdp-inspector, write-commands, extension inspector - Queue file permissions (0o700/0o600) in cli, server, sidebar-agent - escapeRegExp for frame --url ReDoS fix - Responsive screenshot path validation with validateOutputPath - State load cookie filtering (reject localhost/.internal/metadata cookies) - Session ID format validation in loadSession - /health endpoint: remove currentUrl and currentMessage fields - QueueEntry interface + isValidQueueEntry validator for sidebar-agent - SIGTERM->SIGKILL escalation in timeout handler - Viewport dimension clamping (1-16384), wait timeout clamping (1s-300s) - Cookie domain validation in cookie-import and cookie-import-browser - DocumentFragment-based tab switching (XSS fix in sidepanel) - pollInProgress reentrancy guard for pollChat - toggleClass/injectCSS input validation in extension inspector - Snapshot annotated path validation with realpathSync - 714-line security-audit-r2.test.ts + 33-line learnings-injection.test.ts Co-Authored-By: mr-k-man <mr-k-man@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: bump version and changelog (v0.15.13.0) Community security wave: 8 PRs from 4 contributors (@garagon, @mr-k-man, @mmporong, @0531Kim, @pieterklue). IPv6 ULA blocking, cookie redaction, per-tab cancel signaling, CSS injection guards, timeout clamping, session validation, DocumentFragment XSS fix, parent process watchdog, uninstall docs, Windows fixes, and 750+ lines of security regression tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: garagon <garagon@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: 0531Kim <0531Kim@users.noreply.github.com> Co-authored-by: pieterklue <pieterklue@users.noreply.github.com> Co-authored-by: mmporong <mmporong@users.noreply.github.com> Co-authored-by: mr-k-man <mr-k-man@users.noreply.github.com>
This commit is contained in:
Garry Tan
+11
-7
@@ -55,6 +55,10 @@ export async function serve(options: ServeOptions): Promise<void> {
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
// Security: anchor all file reads to the initial HTML's directory.
|
||||
// Prevents /api/reload from reading arbitrary files via path traversal.
|
||||
const allowedDir = fs.realpathSync(path.dirname(path.resolve(html)));
|
||||
|
||||
let htmlContent = fs.readFileSync(html, "utf-8");
|
||||
let state: ServerState = "serving";
|
||||
let timeoutTimer: ReturnType<typeof setTimeout> | null = null;
|
||||
@@ -185,19 +189,19 @@ export async function serve(options: ServeOptions): Promise<void> {
|
||||
);
|
||||
}
|
||||
|
||||
// Validate path is within cwd or temp directory
|
||||
const resolved = path.resolve(newHtmlPath);
|
||||
const safeDirs = [process.cwd(), os.tmpdir()];
|
||||
const isSafe = safeDirs.some(dir => resolved.startsWith(dir + path.sep) || resolved === dir);
|
||||
if (!isSafe) {
|
||||
// Security: resolve symlinks and validate the reload path is within the
|
||||
// allowed directory (anchored to the initial HTML file's parent).
|
||||
// Prevents path traversal via /api/reload reading arbitrary files.
|
||||
const resolvedReload = fs.realpathSync(path.resolve(newHtmlPath));
|
||||
if (!resolvedReload.startsWith(allowedDir + path.sep) && resolvedReload !== allowedDir) {
|
||||
return Response.json(
|
||||
{ error: `Path must be within working directory or temp` },
|
||||
{ error: `Path must be within: ${allowedDir}` },
|
||||
{ status: 403 }
|
||||
);
|
||||
}
|
||||
|
||||
// Swap the HTML content
|
||||
htmlContent = fs.readFileSync(newHtmlPath, "utf-8");
|
||||
htmlContent = fs.readFileSync(resolvedReload, "utf-8");
|
||||
state = "serving";
|
||||
|
||||
console.error(`SERVE_RELOADED: html=${newHtmlPath}`);
|
||||
|
||||
@@ -274,6 +274,103 @@ describe('Serve HTTP endpoints', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Path traversal protection in /api/reload ─────────────────────
|
||||
|
||||
describe('Serve /api/reload — path traversal protection', () => {
|
||||
let server: ReturnType<typeof Bun.serve>;
|
||||
let baseUrl: string;
|
||||
let htmlContent: string;
|
||||
let allowedDir: string;
|
||||
|
||||
beforeAll(() => {
|
||||
// Production-equivalent allowedDir anchored to tmpDir
|
||||
allowedDir = fs.realpathSync(tmpDir);
|
||||
htmlContent = fs.readFileSync(boardHtml, 'utf-8');
|
||||
|
||||
// This server mirrors the production serve() with the path validation fix
|
||||
server = Bun.serve({
|
||||
port: 0,
|
||||
fetch(req) {
|
||||
const url = new URL(req.url);
|
||||
|
||||
if (req.method === 'GET' && url.pathname === '/') {
|
||||
return new Response(htmlContent, {
|
||||
headers: { 'Content-Type': 'text/html; charset=utf-8' },
|
||||
});
|
||||
}
|
||||
|
||||
if (req.method === 'POST' && url.pathname === '/api/reload') {
|
||||
return (async () => {
|
||||
let body: any;
|
||||
try { body = await req.json(); } catch { return Response.json({ error: 'Invalid JSON' }, { status: 400 }); }
|
||||
if (!body.html || !fs.existsSync(body.html)) {
|
||||
return Response.json({ error: `HTML file not found: ${body.html}` }, { status: 400 });
|
||||
}
|
||||
// Production path validation — same as design/src/serve.ts
|
||||
const resolvedReload = fs.realpathSync(path.resolve(body.html));
|
||||
if (!resolvedReload.startsWith(allowedDir + path.sep) && resolvedReload !== allowedDir) {
|
||||
return Response.json({ error: `Path must be within: ${allowedDir}` }, { status: 403 });
|
||||
}
|
||||
htmlContent = fs.readFileSync(resolvedReload, 'utf-8');
|
||||
return Response.json({ reloaded: true });
|
||||
})();
|
||||
}
|
||||
|
||||
return new Response('Not found', { status: 404 });
|
||||
},
|
||||
});
|
||||
baseUrl = `http://localhost:${server.port}`;
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
server.stop();
|
||||
});
|
||||
|
||||
test('blocks reload with path outside allowed directory', async () => {
|
||||
const res = await fetch(`${baseUrl}/api/reload`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ html: '/etc/passwd' }),
|
||||
});
|
||||
expect(res.status).toBe(403);
|
||||
const data = await res.json();
|
||||
expect(data.error).toContain('Path must be within');
|
||||
});
|
||||
|
||||
test('blocks reload with symlink pointing outside allowed directory', async () => {
|
||||
const linkPath = path.join(tmpDir, 'evil-link.html');
|
||||
try {
|
||||
fs.symlinkSync('/etc/passwd', linkPath);
|
||||
const res = await fetch(`${baseUrl}/api/reload`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ html: linkPath }),
|
||||
});
|
||||
expect(res.status).toBe(403);
|
||||
} finally {
|
||||
try { fs.unlinkSync(linkPath); } catch {}
|
||||
}
|
||||
});
|
||||
|
||||
test('allows reload with file inside allowed directory', async () => {
|
||||
const goodPath = path.join(tmpDir, 'safe-board.html');
|
||||
fs.writeFileSync(goodPath, '<html><body>Safe reload</body></html>');
|
||||
|
||||
const res = await fetch(`${baseUrl}/api/reload`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ html: goodPath }),
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
const data = await res.json();
|
||||
expect(data.reloaded).toBe(true);
|
||||
|
||||
// Verify the new content is served
|
||||
const page = await fetch(baseUrl);
|
||||
expect(await page.text()).toContain('Safe reload');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Full lifecycle: regeneration round-trip ──────────────────────
|
||||
|
||||
describe('Full regeneration lifecycle', () => {
|
||||
|
||||
Reference in New Issue
Block a user