v1.34.0.0 feat: gstack consumable as submodule (factory-export API + AUTH_TOKEN env + import.meta.main gate)#1472
Merged
Merged
Conversation
…gletonLocks Three new exported helpers in browse/src/config.ts: - resolveGstackHome(): honors GSTACK_HOME env, falls back to os.homedir()/.gstack Matches the existing convention in browse/src/telemetry.ts:26 and browse/src/domain-skills.ts:66. - resolveChromiumProfile(explicit?): explicit arg wins -> CHROMIUM_PROFILE env -> resolveGstackHome()/chromium-profile. Lets gbrowser pass per-workspace profile paths through ServerConfig instead of relying on ambient env state. - cleanSingletonLocks(dir): removes SingletonLock/Socket/Cookie via safeUnlinkQuiet. Defensive guard refuses to operate unless dir basename is 'chromium-profile' OR matches explicit CHROMIUM_PROFILE env value, preventing accidental deletion in unrelated directories. Extends browse/test/config.test.ts with 12 tests covering env precedence, guard behavior, ENOENT swallowing, and CHROMIUM_PROFILE override. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The checkTranscript Promise executor in browse/src/security-classifier.ts referenced `finish()` at the !claude early-return guard before declaring it 5 lines later. JavaScript throws ReferenceError: Cannot access 'finish' before initialization (TDZ) for that path, but the path is only reachable when resolveClaudeCommand returns null inside the spawn block (a TOCTOU window vs. the outer checkHaikuAvailable cache). Fix: hoist `let stdout = ''`, `let done = false`, and `const finish` block above `const claude = resolveClaudeCommand()` so finish is in scope before any reference to it. Behavior is identical when claude is on PATH; the fix only matters for the dormant missing-CLI degraded path. Adds browse/test/security-classifier-tdz.test.ts as the regression guard: clears PATH + override env vars, calls checkTranscript, asserts the result serializes with degraded:true and a meaningful reason field. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…+ lock cleanup Three fold-ins so gbrowser can become a thin overlay instead of forking browse-server: - Export isCustomChromium(): detects custom Chromium builds that bake the extension in as a component extension. Prefers explicit GSTACK_CHROMIUM_KIND=custom-extension-baked signal; falls back to GSTACK_CHROMIUM_PATH substring containing 'GBrowser' / 'gbrowser'. Gates the --load-extension push at launchHeaded so we don't trigger ServiceWorkerState::SetWorkerId DCHECK when two copies of the same service worker race to register. - Swap hardcoded path.join(HOME, '.gstack', 'chromium-profile') in launchHeaded for resolveChromiumProfile() so phoenix can pass a per-workspace profile via CHROMIUM_PROFILE env (one daemon per gbd workspace, each with a distinct profile dir). - Call cleanSingletonLocks(userDataDir) immediately after mkdirSync. Chromium's ProcessSingleton refuses to start when stale SingletonLock/Socket/Cookie files survive a SIGKILL or hard crash; pre-launch cleanup defends against the crash case. Safe under external coordination (gbd.lock for gbrowser, single-instance CLI check for gstack). The existing .auth.json write at L291-302 is preserved — extensions still need it for bootstrap even when component-baked. Adds browse/test/browser-manager-custom-chromium.test.ts with 8 tests covering both the env-kind and path-substring signals plus stock / playwright-bundled Chromium negative cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Surfaces the embedder API gbrowser (phoenix) needs to consume gstack as a submodule, and gates module-load side effects so the file is safe to import without auto-starting a daemon. Changes to browse/src/server.ts: - AUTH_TOKEN now honors process.env.AUTH_TOKEN (trimmed) before falling back to crypto.randomUUID(). Whitespace-only values are rejected so the security boundary can't be silently weakened. - New exported types: ServerConfig and ServerHandle. ServerConfig documents the full factory contract (authToken, browsePort, idleTimeoutMs, config, browserManager, chromiumProfile, xvfb, proxyBridge, startTime, beforeRoute). ServerHandle documents the return shape (fetchLocal, fetchTunnel, shutdown, stopListeners). Caller-owned lifecycle annotations on xvfb and proxyBridge prevent double-close bugs from surprise ownership. - New exported function: resolveConfigFromEnv() builds a ServerConfig-shaped object from process.env for CLI use. Embedders construct their own ServerConfig explicitly. - start() is now exported. Embedders can call it with env vars set as a v1 escape hatch until full buildFetchHandler extraction lands. - Signal handlers (SIGINT, SIGTERM, Windows exit, uncaughtException, unhandledRejection) and the auto-kickoff at module bottom are now wrapped in `if (import.meta.main)`. CLI path is unchanged. Embedders register their own handlers. - shutdown() and emergencyCleanup() now call cleanSingletonLocks( resolveChromiumProfile()) instead of inline path+loop. Single implementation, defensive guard, honors per-workspace CHROMIUM_PROFILE. New tests: - browse/test/server-no-import-side-effects.test.ts: spawns a fresh Bun subprocess that imports server.ts, asserts no signal handlers registered, no state-dir populated. Guards the core refactor invariant from regression. - browse/test/server-factory.test.ts: 12 tests covering AUTH_TOKEN env behavior (honored, whitespace-rejected, trimmed), preserved exports (TUNNEL_COMMANDS, canDispatchOverTunnel), and ServerConfig/ServerHandle type compatibility. Deferred to follow-up PR: full buildFetchHandler extraction that hoists the 13 module-level mutables + helpers into a factory closure. Phoenix can ship v0.6.0.0 against the start()+env surface today; the cleaner factory comes next. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three security hardening fixes from /ship adversarial review: 1. AUTH_TOKEN unicode-whitespace bypass (server.ts:67-83). Old: `process.env.AUTH_TOKEN?.trim() || randomUUID()` only stripped ASCII whitespace. A misconfigured embedder shipping AUTH_TOKEN=$'' (BOM) or $'' (zero-width space) would silently get a one-character bearer secret. New `sanitizeAuthToken()` strips all unicode whitespace via regex and requires >= 16 chars after stripping; anything shorter falls back to crypto.randomUUID(). Same sanitizer used by `resolveConfigFromEnv()` so the embedder path is hardened too. 2. security-classifier.ts checkTranscript safety net. `resolveClaudeCommand()` and `spawn()` can throw under transient conditions (PATH probe failure, posix_spawn ENOMEM). Old code let the throw propagate and rejected the Promise with a raw exception. Now wrapped in try/catch that calls finish() with a degraded signal, matching the graceful-degradation contract the layer already promises for missing-CLI / exit-nonzero / parse-error. 3. cleanSingletonLocks defensive guard tightened (config.ts). Old: basename === 'chromium-profile' OR userDataDir === $CHROMIUM_PROFILE. The second branch was env-controlled and the first was bypassable by passing a relative path that resolved to chromium-profile via CWD drift. New guard: refuses relative paths outright, resolves both sides via path.resolve(), and only accepts the env-match path when $CHROMIUM_PROFILE is itself absolute. Test updates: replace the old `.trim()` test with three new cases covering unicode-whitespace stripping, short-token rejection, and zero-width-only rejection (server-factory.test.ts). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
E2E Evals: ✅ PASS6/6 tests passed | $.99 total cost | 12 parallel runners
12x ubicloud-standard-2 (Docker: pre-baked toolchain + deps) | wall clock ≈ slowest suite |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GStack is now consumable as a submodule. Five new exported helpers +
AUTH_TOKENenv injection +import.meta.maingate let downstream Bun projects embed the browse server without forking. Plus 3 security hardening fixes from adversarial review and a TDZ regression bug fix.The whole release in one screen: see CHANGELOG.md
## [1.34.0.0].Shipped in 5 commits
Foundation (4 commits):
bed1a9f5config.ts helpers:resolveGstackHome(honorsGSTACK_HOME),resolveChromiumProfile(explicit?),cleanSingletonLockswith defensive guard.4b3bbed2security-classifier TDZ fix:finish()hoisted aboveresolveClaudeCommand()+ regression test (IRON RULE).fde757cfbrowser-manager fold-in:isCustomChromium()exported,--load-extensiongated, profile viaresolveChromiumProfile(),cleanSingletonLockspre-launch.df6d2a16server.ts factory-export API:ServerConfig/ServerHandletypes,resolveConfigFromEnv(),start()exported,AUTH_TOKENenv injection,import.meta.maingates,cleanSingletonLockswired into shutdown/emergencyCleanup.Adversarial hardening (1 commit):
149fe743fix: harden auth-token validation (unicode-whitespace bypass), TDZ try/catch, lockfile path safety. Three issues caught by /ship adversarial review before merge.Test Coverage
Pre-merge coverage audit ran as a subagent. 94% effective coverage across the new surface. 3 GAPs deferred — all require real-browser launch (covered by dev smoke).
resolveGstackHome()resolveChromiumProfile(explicit?)cleanSingletonLocks(userDataDir)isCustomChromium()predicatecheckTranscriptTDZ hoistAUTH_TOKENenv path / sanitizationresolveConfigFromEnv()ServerConfig/ServerHandle/SurfacetypesTUNNEL_COMMANDS/canDispatchOverTunnelpreservedimport.meta.maingate (no auto-start)launchHeadedextension/profile/lock branchesstart()export from non-main callerTests: 5 new test files + 1 extended. 34 new tests. Final test pass: 96/96 critical tests green.
Pre-Landing Review
0 findings at confidence >= 7. Clean refactor: factory-export API surface, env-injectable
AUTH_TOKENwith safe whitespace fallback,import.meta.maingating, sharedcleanSingletonLockshelper, TDZ fix with regression test.Adversarial Review
5 findings from Claude adversarial pass. 3 auto-fixed in commit
149fe743:AUTH_TOKEN .trim()only strips ASCII whitespace — BOM (U+FEFF) or zero-width (U+200B) tokens pass as one-character bearer secretssanitizeAuthToken()strips all unicode whitespace + requires >= 16 charsresolveClaudeCommand()/spawn()can throw — Promise executor rejected with raw exceptioncleanSingletonLocksaccepted CWD-relative paths and env-controlled paths that bypassed basename guardpath.resolve(), env-match requires absoluteCHROMIUM_PROFILE2 informational findings accepted (substring-match looseness on
isCustomChromiumand embedder-contract risk inbeforeRoutedocstring — neither is a boundary violation, both documented for follow-up).Codex adversarial run skipped this round for time; Codex plan-review was already run during /plan-eng-review and surfaced D9-D13 cross-model tensions resolved in the plan.
Plan Completion
Plan completion audit (subagent): 18 DONE + 5 CHANGED + 0 NOT DONE + 0 UNVERIFIABLE. CHANGED items are the explicitly-deferred
buildFetchHandlerruntime extraction — documented in the JSDoc at server.ts ~L113 and the commit message ofdf6d2a16. Phoenix can ship v0.6.0.0 today against the start()+env surface; the cleaner factory lands in a follow-up.Pre-existing failures (NOT my changes)
test/gstack-next-version.test.ts(CLI runs against real repo and emits parseable JSON) times out at 5s. Verified the same failure exists onorigin/main(dc6252d1). Pre-existing — not introduced by this branch. Tracking separately.TODOS
TODOS.md has 0 items completed by this PR. No new TODOs added — the deferred
buildFetchHandlerextraction is documented in the plan file and JSDoc, not as a project TODO.Documentation
No README/CLAUDE.md doc updates needed — diff is internal browse server API for embedders, not user-facing CLI surface.
Test plan
bun test browse/test/<critical-files>)bun run buildpasses (tsc + binary compile)bun run dev goto https://example.com && bun run dev text && bun run dev stopworks end-to-endimport 'server.ts'doesn't auto-start (subprocess assert)import { start }integration actually work? Smoke test on the gbrowser side once landed.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmithwith what you need.