diff --git a/browse/src/browse-client.ts b/browse/src/browse-client.ts index a33681f717..435f575469 100644 --- a/browse/src/browse-client.ts +++ b/browse/src/browse-client.ts @@ -54,6 +54,13 @@ interface ResolvedAuth { source: 'env' | 'state-file'; } +function parseIntegerEnvValue(value: string | undefined): number | undefined { + const trimmed = value?.trim(); + if (!trimmed || !/^-?\d+$/.test(trimmed)) return undefined; + const parsed = parseInt(trimmed, 10); + return Number.isFinite(parsed) ? parsed : undefined; +} + /** Resolve the daemon port + token. Throws a clear error if neither path works. */ export function resolveBrowseAuth(opts: BrowseClientOptions = {}): ResolvedAuth { if (opts.port !== undefined && opts.token !== undefined) { @@ -64,8 +71,8 @@ export function resolveBrowseAuth(opts: BrowseClientOptions = {}): ResolvedAuth const envPort = process.env.GSTACK_PORT; const envToken = process.env.GSTACK_SKILL_TOKEN; if (envPort && envToken) { - const port = opts.port ?? parseInt(envPort, 10); - if (!isNaN(port)) { + const port = opts.port ?? parseIntegerEnvValue(envPort); + if (port !== undefined) { return { port, token: opts.token ?? envToken, source: 'env' }; } } @@ -132,7 +139,7 @@ export class BrowseClient { const auth = resolveBrowseAuth(opts); this.port = auth.port; this.token = auth.token; - this.tabId = opts.tabId ?? (process.env.BROWSE_TAB ? parseInt(process.env.BROWSE_TAB, 10) : undefined); + this.tabId = opts.tabId ?? parseIntegerEnvValue(process.env.BROWSE_TAB); this.timeoutMs = opts.timeoutMs ?? 30_000; } diff --git a/browse/test/browse-client.test.ts b/browse/test/browse-client.test.ts index 1def4a8833..61853f9941 100644 --- a/browse/test/browse-client.test.ts +++ b/browse/test/browse-client.test.ts @@ -87,6 +87,18 @@ describe('browse-client', () => { expect(auth.source).toBe('env'); }); + it('rejects GSTACK_PORT env values with trailing characters', () => { + process.env.GSTACK_PORT = `${server.port}abc`; + process.env.GSTACK_SKILL_TOKEN = 'scoped-token'; + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-client-test-')); + try { + expect(() => resolveBrowseAuth({ stateFile: path.join(tmpDir, 'missing.json') })) + .toThrow('browse-client: cannot find daemon port + token'); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + it('falls back to state file when env vars missing', () => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-client-test-')); const stateFile = path.join(tmpDir, 'browse.json'); @@ -154,6 +166,13 @@ describe('browse-client', () => { expect(server.requests[0].body).toEqual({ command: 'text', args: [], tabId: 7 }); }); + it('omits tabId when BROWSE_TAB has trailing characters', async () => { + process.env.BROWSE_TAB = '7abc'; + const client = new BrowseClient({ port: server.port, token: 't' }); + await client.command('text', []); + expect(server.requests[0].body).toEqual({ command: 'text', args: [] }); + }); + it('throws BrowseClientError with status on non-2xx', async () => { const client = new BrowseClient({ port: server.port, token: 't' }); server.setResponse(403, JSON.stringify({ error: 'Insufficient scope' })); diff --git a/browser-skills/hackernews-frontpage/_lib/browse-client.ts b/browser-skills/hackernews-frontpage/_lib/browse-client.ts index a33681f717..435f575469 100644 --- a/browser-skills/hackernews-frontpage/_lib/browse-client.ts +++ b/browser-skills/hackernews-frontpage/_lib/browse-client.ts @@ -54,6 +54,13 @@ interface ResolvedAuth { source: 'env' | 'state-file'; } +function parseIntegerEnvValue(value: string | undefined): number | undefined { + const trimmed = value?.trim(); + if (!trimmed || !/^-?\d+$/.test(trimmed)) return undefined; + const parsed = parseInt(trimmed, 10); + return Number.isFinite(parsed) ? parsed : undefined; +} + /** Resolve the daemon port + token. Throws a clear error if neither path works. */ export function resolveBrowseAuth(opts: BrowseClientOptions = {}): ResolvedAuth { if (opts.port !== undefined && opts.token !== undefined) { @@ -64,8 +71,8 @@ export function resolveBrowseAuth(opts: BrowseClientOptions = {}): ResolvedAuth const envPort = process.env.GSTACK_PORT; const envToken = process.env.GSTACK_SKILL_TOKEN; if (envPort && envToken) { - const port = opts.port ?? parseInt(envPort, 10); - if (!isNaN(port)) { + const port = opts.port ?? parseIntegerEnvValue(envPort); + if (port !== undefined) { return { port, token: opts.token ?? envToken, source: 'env' }; } } @@ -132,7 +139,7 @@ export class BrowseClient { const auth = resolveBrowseAuth(opts); this.port = auth.port; this.token = auth.token; - this.tabId = opts.tabId ?? (process.env.BROWSE_TAB ? parseInt(process.env.BROWSE_TAB, 10) : undefined); + this.tabId = opts.tabId ?? parseIntegerEnvValue(process.env.BROWSE_TAB); this.timeoutMs = opts.timeoutMs ?? 30_000; }