fix(security): 20 additional bugs found via sqry semantic code graph (round 2) by mr-k-man · Pull Request #806 · garrytan/gstack

mr-k-man · 2026-04-05T02:41:49Z

Summary

Second security audit of gstack's browse subsystem, Chrome extension, and CLI scripts — all findings beyond the 10 already reported in PR #664 and issues #665-#675.

20 vulnerabilities fixed across 13 files (1 HIGH, 1 MEDIUM-HIGH, 14 MEDIUM, 4 LOW)
119 regression tests (source-level + behavioral)
Method: sqry AST-based semantic code graph analysis + 4 parallel audit agents + 9 rounds of Codex plan review + mid-implementation code review

Findings by severity

#	Severity	Finding	File(s)
1	HIGH	Shell injection via bash-to-JS interpolation in `bin/gstack-learnings-search`	`bin/gstack-learnings-search`
2	MED-HIGH	Agent queue file poisoning — default permissions + no schema validation	`server.ts`, `sidebar-agent.ts`, `cli.ts`
3	MED	`/health` endpoint leaks `currentUrl` and `currentMessage` without auth	`server.ts`
4	MED	ReDoS via `new RegExp(args[1])` in `frame --url`	`meta-commands.ts`
5	MED	`chain` command bypasses watch-mode write guard	`meta-commands.ts`
6	MED	`cookie-import` allows cross-domain cookie planting (arbitrary `domain` field)	`write-commands.ts`
7	MED	CSS injection via `style` command value — no validation at 4 injection points	`write-commands.ts`, `cdp-inspector.ts`, `inspector.js`
8	MED	Session directory traversal via crafted `active.json` ID	`server.ts`
9	MED	`responsive` generated filenames skip `validateOutputPath`	`meta-commands.ts`
10	MED	`validateOutputPath` uses `path.resolve` not `realpathSync` — symlink bypass	`meta-commands.ts`, `write-commands.ts`, `snapshot.ts`
11	MED	`state load` restores cookies + navigates page URLs without validation	`meta-commands.ts`, `browser-manager.ts`
12	MED	`chatDomByTab` DOM serialization round-trip XSS in tab switch	`sidepanel.js`
13	MED	`switchChatTab` / `pollChat` mutual recursion (stack overflow)	`sidepanel.js`
14	MED	`cookie-import-browser --domain` accepts unvalidated domain	`write-commands.ts`
15	LOW-MED	Unsanitized `activeTabUrl` passed to `syncActiveTabByUrl` (2 call sites)	`server.ts`
16	LOW-MED	`inbox` outputs unvalidated user-controlled strings (prompt injection surface)	`meta-commands.ts`
17	LOW	Sidebar agent timeout handler missing SIGKILL escalation	`sidebar-agent.ts`
18	LOW	`viewport` accepts unbounded dimensions (OOM via multi-GB screenshots)	`write-commands.ts`
19	LOW	`wait` timeout has no upper bound (event loop block)	`write-commands.ts`
20	LOW	`stateFile` in queue entry missing path traversal check	`sidebar-agent.ts`

How we found them

Built sqry semantic code graph index (43,603 nodes, 36,355 edges, 188 files)
Queried for dangerous sinks (`exec`, `spawn`, `token`, `password`, `secret`), complexity hotspots, and call cycles
Dispatched 4 parallel audit agents targeting `server.ts`, `write-commands.ts`, `meta-commands.ts`, and `extension/` code
Cross-referenced all 25 raw findings against existing issues/PRs — 16 were genuinely new
Codex reviewed implementation plan through 9 rounds until unconditional APPROVE
Mid-implementation code review caught 4 additional gaps (all fixed)

Cross-validation with other community reports

7 of our original 10 findings from PR #664 were independently rediscovered by stedfn (#712-#717) and Gonzih (#741-#758). Several of these round 2 findings overlap with seanomich's audit (#783) but target different specific vectors.

Test results

Security regression tests (119/119 pass)

```
119 pass, 0 fail, 182 expect() calls
Ran 119 tests across 5 files [47ms]
```

E2E evals (33 pass, 0 regressions)

Ran in `gstack-ci` Docker image (Ubuntu 24.04 + Chromium 145/Playwright 1.58.2 + `--cap-add SYS_ADMIN`):

All previously-failing browse tests now pass:

Test	Result	Duration
browse-basic	PASS	12s
browse-snapshot	PASS	19s
qa-fix-loop	PASS	175s
browse qa-quick	PASS	84s
browse qa-only-no-fix	PASS	136s
browse qa-b6-static	PASS	111s
browse qa-b7-spa	PASS	158s
browse qa-b8-checkout	PASS	178s
sidebar-url-accuracy	PASS	<1s
skillmd-setup-discovery	PASS	6s
skillmd-no-local-binary	PASS	7s
skillmd-outside-git	PASS	5s
session-awareness	PASS	10s
operational-learning	PASS	7s
ship-local-workflow	PASS	15s
ship-coverage-audit	PASS	27s
document-release	PASS	46s
canary-workflow	PASS	34s
benchmark-workflow	PASS	49s
setup-deploy-workflow	PASS	18s
gstack-upgrade-happy-path	PASS	29s
codex-review	PASS	<1s
land-and-deploy (3 tests)	PASS	52-63s
review-army (6 tests)	PASS	11-53s
timeline-event-flow	PASS	<1s

1 unrelated failure (`qa-bootstrap` — test infrastructure, not our code).

Test plan

119 security regression tests pass (source-level + behavioral)
33 E2E evals pass including all browse/snapshot/QA tests
browse-basic, browse-snapshot, qa-fix-loop all pass (previously failing)
All existing path-validation, url-validation, and server-auth tests pass
No new regressions vs origin/main

…ymlink traversal Both meta-commands.ts and write-commands.ts validateOutputPath previously used path.resolve which resolves paths logically without following symlinks. A symlink at /tmp/safe pointing to /etc would pass the safe-directory check. Switched to fs.realpathSync (matching the existing validateReadPath pattern in read-commands.ts): - Resolve the full real path for existing files - Resolve the parent directory for new files (ENOENT case) - Also resolve SAFE_DIRECTORIES themselves through realpathSync (handles macOS /tmp → /private/tmp) Added browse/test/security-audit-r2.test.ts with source-level checks and behavioral tests including symlink-to-/etc attack verification.

Block data exfiltration via url(), expression(), @import, javascript:, and data: patterns in write-commands style handler, cdp-inspector modifyStyle, extension injectCSS (also validates id format), and extension toggleClass (validates className format). Adds source-level regression tests for all four injection points.

Replace direct bash variable interpolation into JS string literals with environment variable passing. Values are now passed via GSTACK_FILTER_* env vars and read with process.env inside the bun -e block, eliminating the shell injection vector where single quotes in --query/--type/--slug could break out of JS string literals and execute arbitrary code. Adds browse/test/learnings-injection.test.ts with source-level and behavioral tests to prevent regression.

…ions Add isValidQueueEntry() validator to sidebar-agent.ts that checks all 8 QueueEntry fields (prompt required, args array, cwd path traversal, typed optional fields). Invalid entries are skipped with console.warn. Queue directory and file permissions hardened to 0o700/0o600 in all three writers: server.ts, sidebar-agent.ts, and cli.ts. Tests added to security-audit-r2.test.ts.

- applyStyle in extension/inspector.js now validates CSS values with the same DANGEROUS_CSS pattern as injectCSS (blocks url(), expression(), @import, javascript:, data: exfiltration vectors) - snapshot.ts annotated screenshot path validation now uses realpathSync to resolve symlinks before checking against safe directories - cookie-import in write-commands.ts replaces inline path.resolve+isPathWithin with realpathSync-based validation consistent with validateOutputPath - isValidQueueEntry in sidebar-agent.ts now checks stateFile for '..' path traversal sequences in addition to type validation - Adds source-level regression tests for all four fixes in security-audit-r2.test.ts

…ted /health

…port-browser

… in tab switch

…ntrancy guard

…timeouts

garrytan · 2026-04-06T06:31:49Z

Big thanks for this contribution, @mr-k-man! Your fix has been landed on the garrytan/security-wave-5 branch as part of our community security wave (commit dfe946fe). We credited you as co-author. Ships in v0.15.12.0. Really appreciate you taking the time to find and fix this.

@mr-k-man

…ion, 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>

@0531Kim

* 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>

sqry-release-plz bot added 16 commits April 5, 2026 02:00

fix(security): remove currentUrl and currentMessage from unauthentica…

dcdae09

…ted /health

fix(security): escape user input in frame --url to prevent ReDoS

4e8c644

fix(security): validate cookie domains in cookie-import and cookie-im…

ed7f169

…port-browser

fix(security): validate each responsive screenshot path, not just prefix

5cf8cad

fix(security): validate cookies and page URLs in state load

cec3d06

fix(security): validate activeTabUrl before syncActiveTabByUrl

2733610

fix(security): wrap inbox output as untrusted content

ff6b1b5

fix(security): replace DOM serialization round-trip with node cloning…

75161ad

… in tab switch

fix(security): break switchChatTab/pollChat mutual recursion with ree…

481b3f8

…ntrancy guard

fix(security): add SIGKILL escalation to sidebar-agent timeout handler

e966c75

fix(security): add bounds validation to viewport dimensions and wait …

c902fe2

…timeouts

Ziadstr mentioned this pull request Apr 5, 2026

fix(security): input validation, prompt injection defense, and shell escape in learnings system #841

Open

garrytan closed this Apr 6, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(security): 20 additional bugs found via sqry semantic code graph (round 2)#806

fix(security): 20 additional bugs found via sqry semantic code graph (round 2)#806
mr-k-man wants to merge 16 commits intogarrytan:mainfrom
mr-k-man:fix/security-audit-round2

mr-k-man commented Apr 5, 2026 •

edited

Loading

Uh oh!

garrytan commented Apr 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mr-k-man commented Apr 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Findings by severity

How we found them

Cross-validation with other community reports

Test results

Security regression tests (119/119 pass)

E2E evals (33 pass, 0 regressions)

Test plan

Uh oh!

garrytan commented Apr 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mr-k-man commented Apr 5, 2026 •

edited

Loading