Merge PR #1339: reject partial browse client env integers

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);
+    const port = opts.port ?? parseIntegerEnvValue(envPort);
-    if (!isNaN(port)) {
+    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;
   }
 

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' }));

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);
+    const port = opts.port ?? parseIntegerEnvValue(envPort);
-    if (!isNaN(port)) {
+    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;
   }