mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-02 03:35:09 +02:00
fix: CSO security fixes — token leak, domain bypass, input validation
1. Remove root token from /health endpoint entirely (CSO #1 CRITICAL). Origin header is spoofable. Extension reads from ~/.gstack/.auth.json. 2. Add domain check for newtab URL (CSO #5). Previously only goto was checked, allowing domain-restricted agents to bypass via newtab. 3. Validate scope values, rateLimit, expiresSeconds in createToken() (CSO #4). Rejects invalid scopes and negative values. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
+14
-5
@@ -893,6 +893,16 @@ async function handleCommand(body: any, tokenInfo?: TokenInfo | null): Promise<R
|
||||
|
||||
// ─── newtab with ownership for scoped tokens ──────────────
|
||||
if (command === 'newtab' && tokenInfo && tokenInfo.clientId !== 'root') {
|
||||
// Domain check for newtab URL (same as goto)
|
||||
if (args[0] && !checkDomain(tokenInfo, args[0])) {
|
||||
return new Response(JSON.stringify({
|
||||
error: 'Domain not allowed by your token scope',
|
||||
hint: `Allowed domains: ${tokenInfo.domains?.join(', ') || 'none configured'}`,
|
||||
}), {
|
||||
status: 403,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
});
|
||||
}
|
||||
const newId = await browserManager.newTab(args[0] || undefined, tokenInfo.clientId);
|
||||
return new Response(JSON.stringify({
|
||||
tabId: newId,
|
||||
@@ -1201,11 +1211,10 @@ async function start() {
|
||||
uptime: Math.floor((Date.now() - startTime) / 1000),
|
||||
tabs: browserManager.getTabCount(),
|
||||
currentUrl: browserManager.getCurrentUrl(),
|
||||
// Auth token for extension bootstrap. Only returned when the request
|
||||
// comes from a Chrome extension (Origin: chrome-extension://...).
|
||||
// Previously served unconditionally, but that leaks the token if the
|
||||
// server is tunneled to the internet (ngrok, SSH tunnel).
|
||||
...(req.headers.get('origin')?.startsWith('chrome-extension://') ? { token: AUTH_TOKEN } : {}),
|
||||
// Auth token NOT served here. Extension reads from ~/.gstack/.auth.json
|
||||
// (written by launchHeaded at browser-manager.ts:243). Serving the token
|
||||
// on an unauthenticated endpoint is unsafe because Origin headers are
|
||||
// trivially spoofable, and ngrok exposes /health to the internet.
|
||||
chatEnabled: true,
|
||||
agent: {
|
||||
status: agentStatus,
|
||||
|
||||
@@ -169,6 +169,18 @@ export function createToken(opts: CreateTokenOptions): TokenInfo {
|
||||
expiresSeconds = 86400, // 24h default
|
||||
} = opts;
|
||||
|
||||
// Validate inputs
|
||||
const validScopes: ScopeCategory[] = ['read', 'write', 'admin', 'meta'];
|
||||
for (const s of scopes) {
|
||||
if (!validScopes.includes(s as ScopeCategory)) {
|
||||
throw new Error(`Invalid scope: ${s}. Valid: ${validScopes.join(', ')}`);
|
||||
}
|
||||
}
|
||||
if (rateLimit < 0) throw new Error('rateLimit must be >= 0');
|
||||
if (expiresSeconds !== null && expiresSeconds !== undefined && expiresSeconds < 0) {
|
||||
throw new Error('expiresSeconds must be >= 0 or null');
|
||||
}
|
||||
|
||||
const token = generateToken('gsk_sess_');
|
||||
const now = new Date();
|
||||
const expiresAt = expiresSeconds === null
|
||||
|
||||
@@ -21,13 +21,28 @@ function sliceBetween(source: string, startMarker: string, endMarker: string): s
|
||||
}
|
||||
|
||||
describe('Server auth security', () => {
|
||||
// Test 1: /health serves auth token gated on chrome-extension:// Origin
|
||||
// to prevent leaking when the server is tunneled to the internet.
|
||||
test('/health serves auth token only for chrome extension origin', () => {
|
||||
// Test 1: /health must NOT serve the auth token (CSO finding #1 — spoofable Origin)
|
||||
// Extension reads token from ~/.gstack/.auth.json instead.
|
||||
test('/health does NOT serve auth token', () => {
|
||||
const healthBlock = sliceBetween(SERVER_SRC, "url.pathname === '/health'", "url.pathname === '/connect'");
|
||||
expect(healthBlock).toContain('AUTH_TOKEN');
|
||||
// Must be gated on chrome-extension Origin
|
||||
expect(healthBlock).toContain('chrome-extension://');
|
||||
// Token must not appear in the health response construction
|
||||
expect(healthBlock).not.toContain('token: AUTH_TOKEN');
|
||||
expect(healthBlock).not.toContain('token: AUTH');
|
||||
// Should have a comment explaining why
|
||||
expect(healthBlock).toContain('NOT served here');
|
||||
});
|
||||
|
||||
// Test 1b: /health must not use chrome-extension Origin gating (spoofable)
|
||||
test('/health does not use spoofable Origin header for token gating', () => {
|
||||
const healthBlock = sliceBetween(SERVER_SRC, "url.pathname === '/health'", "url.pathname === '/connect'");
|
||||
expect(healthBlock).not.toContain("chrome-extension://') ? { token");
|
||||
});
|
||||
|
||||
// Test 1c: newtab must check domain restrictions (CSO finding #5)
|
||||
test('newtab enforces domain restrictions', () => {
|
||||
const newtabBlock = sliceBetween(SERVER_SRC, "newtab with ownership for scoped tokens", "Block mutation commands while watching");
|
||||
expect(newtabBlock).toContain('checkDomain');
|
||||
expect(newtabBlock).toContain('Domain not allowed');
|
||||
});
|
||||
|
||||
// Test 2: /refs endpoint requires auth via validateAuth
|
||||
|
||||
@@ -139,8 +139,11 @@ describe('token-registry', () => {
|
||||
expect(validateToken('gsk_sess_unknown')).toBeNull();
|
||||
});
|
||||
|
||||
it('rejects expired token', () => {
|
||||
const created = createToken({ clientId: 'expiring', expiresSeconds: -1 });
|
||||
it('rejects expired token', async () => {
|
||||
// expiresSeconds: 0 creates a token that expires at creation time
|
||||
const created = createToken({ clientId: 'expiring', expiresSeconds: 0 });
|
||||
// Wait 1ms so the expiry is definitively in the past
|
||||
await new Promise(r => setTimeout(r, 2));
|
||||
expect(validateToken(created.token)).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -345,4 +348,52 @@ describe('token-registry', () => {
|
||||
expect(SCOPE_ADMIN.has('screenshot')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── CSO Fix #4: Input validation ──────────────────────────────
|
||||
describe('Input validation (CSO finding #4)', () => {
|
||||
it('rejects invalid scope values', () => {
|
||||
expect(() => createToken({
|
||||
clientId: 'test-invalid-scope',
|
||||
scopes: ['read', 'bogus' as any],
|
||||
})).toThrow('Invalid scope: bogus');
|
||||
});
|
||||
|
||||
it('rejects negative rateLimit', () => {
|
||||
expect(() => createToken({
|
||||
clientId: 'test-neg-rate',
|
||||
rateLimit: -1,
|
||||
})).toThrow('rateLimit must be >= 0');
|
||||
});
|
||||
|
||||
it('rejects negative expiresSeconds', () => {
|
||||
expect(() => createToken({
|
||||
clientId: 'test-neg-expire',
|
||||
expiresSeconds: -100,
|
||||
})).toThrow('expiresSeconds must be >= 0 or null');
|
||||
});
|
||||
|
||||
it('accepts null expiresSeconds (indefinite)', () => {
|
||||
const token = createToken({
|
||||
clientId: 'test-indefinite',
|
||||
expiresSeconds: null,
|
||||
});
|
||||
expect(token.expiresAt).toBeNull();
|
||||
});
|
||||
|
||||
it('accepts zero rateLimit (unlimited)', () => {
|
||||
const token = createToken({
|
||||
clientId: 'test-unlimited-rate',
|
||||
rateLimit: 0,
|
||||
});
|
||||
expect(token.rateLimit).toBe(0);
|
||||
});
|
||||
|
||||
it('accepts valid scopes', () => {
|
||||
const token = createToken({
|
||||
clientId: 'test-valid-scopes',
|
||||
scopes: ['read', 'write', 'admin', 'meta'],
|
||||
});
|
||||
expect(token.scopes).toEqual(['read', 'write', 'admin', 'meta']);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user