mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-21 12:20:48 +02:00
harden clean-host generation and shutdown
This commit is contained in:
+28
-8
@@ -1571,18 +1571,33 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle {
|
||||
// Factory-scoped validateAuth. Closes over cfg.authToken so every internal
|
||||
// auth check sees the same token the routes receive. Module-level
|
||||
// validateAuth was deleted in v1.35.0.0.
|
||||
let acceptingRequests = true;
|
||||
function validateAuth(req: Request): boolean {
|
||||
const header = req.headers.get('authorization');
|
||||
return header === `Bearer ${authToken}`;
|
||||
return acceptingRequests && header === `Bearer ${authToken}`;
|
||||
}
|
||||
|
||||
// Factory-scoped shutdown. Closes the cfg-provided browserManager so
|
||||
// embedders that pass their own BrowserManager get correct teardown.
|
||||
// Module-level shutdown was deleted in v1.35.0.0.
|
||||
async function shutdown(exitCode: number = 0) {
|
||||
if (isShuttingDown) return;
|
||||
if (!acceptingRequests || isShuttingDown) return;
|
||||
// Close the in-memory authorization gate before deleting discovery state
|
||||
// or awaiting teardown. Existing listeners may remain bound briefly while
|
||||
// Chromium flushes, but no new request can use the root/scoped token or
|
||||
// reach an unauthenticated endpoint that returns the root token.
|
||||
acceptingRequests = false;
|
||||
isShuttingDown = true;
|
||||
|
||||
// Revoke the root bearer before the first await. A SIGINT can terminate
|
||||
// the Bun process while buffer flushing or Chromium teardown is still in
|
||||
// flight; leaving browse.json until the end strands a live credential for
|
||||
// a daemon that no longer exists. The path must come from this factory's
|
||||
// config so embedded/isolated servers never clean a sibling session.
|
||||
const shutdownStateFile = cfg.config.stateFile;
|
||||
const shutdownStateDir = path.dirname(shutdownStateFile);
|
||||
safeUnlinkQuiet(shutdownStateFile);
|
||||
|
||||
console.log('[browse] Shutting down...');
|
||||
if (ownsTerminalAgent) {
|
||||
// Identity-based kill (v1.44+). Replaces the v1.43- `pkill -f
|
||||
@@ -1590,15 +1605,14 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle {
|
||||
// sessions on the same host. Only the PID recorded in
|
||||
// `<stateDir>/terminal-agent-pid` by THIS daemon's agent is signaled.
|
||||
try {
|
||||
const stateDir = path.dirname(config.stateFile);
|
||||
const record = readAgentRecord(stateDir);
|
||||
const record = readAgentRecord(shutdownStateDir);
|
||||
if (record) killAgentByRecord(record, 'SIGTERM');
|
||||
} catch (err: any) {
|
||||
console.warn('[browse] Failed to kill terminal-agent:', err.message);
|
||||
}
|
||||
safeUnlinkQuiet(path.join(path.dirname(config.stateFile), 'terminal-port'));
|
||||
safeUnlinkQuiet(path.join(path.dirname(config.stateFile), 'terminal-internal-token'));
|
||||
safeUnlinkQuiet(agentRecordPath(path.dirname(config.stateFile)));
|
||||
safeUnlinkQuiet(path.join(shutdownStateDir, 'terminal-port'));
|
||||
safeUnlinkQuiet(path.join(shutdownStateDir, 'terminal-internal-token'));
|
||||
safeUnlinkQuiet(agentRecordPath(shutdownStateDir));
|
||||
}
|
||||
try { detachSession(); } catch (err: any) {
|
||||
console.warn('[browse] Failed to detach CDP session:', err.message);
|
||||
@@ -1613,7 +1627,7 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle {
|
||||
await cfgBrowserManager.close();
|
||||
|
||||
cleanSingletonLocks(resolveChromiumProfile());
|
||||
safeUnlinkQuiet(config.stateFile);
|
||||
safeUnlinkQuiet(shutdownStateFile);
|
||||
process.exit(exitCode);
|
||||
}
|
||||
|
||||
@@ -1667,6 +1681,12 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle {
|
||||
|
||||
|
||||
const makeFetchHandler = (surface: Surface) => async (req: Request): Promise<Response> => {
|
||||
if (!acceptingRequests) {
|
||||
return new Response(JSON.stringify({ error: 'Shutting down' }), {
|
||||
status: 503,
|
||||
headers: { 'Content-Type': 'application/json', 'Connection': 'close' },
|
||||
});
|
||||
}
|
||||
const url = new URL(req.url);
|
||||
|
||||
// ─── Tunnel surface filter (runs before any route dispatch) ──
|
||||
|
||||
@@ -13,6 +13,7 @@ import { BrowserManager } from '../src/browser-manager';
|
||||
import { resolveConfig } from '../src/config';
|
||||
import * as crypto from 'crypto';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
|
||||
/**
|
||||
@@ -238,6 +239,54 @@ describe('buildFetchHandler factory contract', () => {
|
||||
expect(typeof handle.stopListeners).toBe('function');
|
||||
});
|
||||
|
||||
test('shutdown revokes its credential state before awaiting browser teardown', async () => {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-shutdown-revoke-'));
|
||||
const stateFile = path.join(root, '.gstack', 'browse.json');
|
||||
fs.mkdirSync(path.dirname(stateFile), { recursive: true });
|
||||
fs.writeFileSync(stateFile, JSON.stringify({ token: 'must-not-survive-sigint' }), { mode: 0o600 });
|
||||
|
||||
let releaseClose: (() => void) | undefined;
|
||||
const slowBrowserManager = {
|
||||
...makeMockBrowserManager('launched'),
|
||||
close: () => new Promise<void>((resolve) => { releaseClose = resolve; }),
|
||||
};
|
||||
const exitMock = mock((_code?: number) => {});
|
||||
const originalExit = process.exit;
|
||||
(process as any).exit = exitMock;
|
||||
__testInternals__.resetShutdownState();
|
||||
try {
|
||||
const handle = buildFetchHandler(makeMinimalConfig({
|
||||
config: resolveConfig({ BROWSE_STATE_FILE: stateFile }),
|
||||
browserManager: slowBrowserManager as any,
|
||||
}));
|
||||
const pendingShutdown = handle.shutdown();
|
||||
|
||||
expect(fs.existsSync(stateFile)).toBe(false);
|
||||
const duringShutdown = await handle.fetchLocal(new Request('http://localhost/refs', {
|
||||
headers: { authorization: 'Bearer must-not-survive-sigint' },
|
||||
}), {});
|
||||
expect(duringShutdown.status).toBe(503);
|
||||
expect(await duringShutdown.json()).toEqual({ error: 'Shutting down' });
|
||||
const healthDuringShutdown = await handle.fetchLocal(
|
||||
new Request('http://localhost/health'),
|
||||
{},
|
||||
);
|
||||
expect(healthDuringShutdown.status).toBe(503);
|
||||
expect(await healthDuringShutdown.text()).not.toContain('must-not-survive-sigint');
|
||||
for (let attempt = 0; attempt < 20 && !releaseClose; attempt += 1) {
|
||||
await new Promise<void>((resolve) => setImmediate(resolve));
|
||||
}
|
||||
expect(releaseClose).toBeDefined();
|
||||
releaseClose!();
|
||||
await pendingShutdown;
|
||||
expect(exitMock).toHaveBeenCalledWith(0);
|
||||
} finally {
|
||||
__testInternals__.resetShutdownState();
|
||||
(process as any).exit = originalExit;
|
||||
fs.rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('2a. cfg.authToken authenticates /health (positive — bearer accepted)', async () => {
|
||||
const cfg = makeMinimalConfig();
|
||||
const handle = buildFetchHandler(cfg);
|
||||
|
||||
Reference in New Issue
Block a user