Skip to content

Commit c8c57ce

Browse files
khaliqgantProactive Runtime Botgithub-actions[bot]
authored
fix(cloud): tolerate stray CLI login callbacks (#1009)
* fix(cloud): tolerate stray CLI login callbacks * style: auto-format with Prettier * fix(cloud): tolerate stray CLI login callbacks * style: auto-format with Prettier --------- Co-authored-by: Proactive Runtime Bot <agent@agent-relay.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent 9bd45de commit c8c57ce

3 files changed

Lines changed: 66 additions & 9 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
5050

5151
### Fixed
5252

53+
- `@agent-relay/cloud`: CLI browser login ignores stray localhost callbacks with an invalid state parameter, so first-time sign-ins are not shown a false hosted error or aborted before the real OAuth callback returns.
5354
- `agent-relay-broker` harness configs now report harness PIDs instead of wrapper worker PIDs, validate app-server protocol/auth/host settings at spawn, and give app-server release requests time to finish.
5455
- `@agent-relay/sdk` normalizes broker `pid: null` spawn responses to `undefined` while PTY harness PIDs are reported asynchronously.
5556
- `web`: PR preview SST deploys use and comment the generated CloudFront URL and AWS's managed disabled cache policy instead of creating per-preview Cloudflare DNS records, ACM certificates, and custom CloudFront cache policies.

packages/cloud/src/auth.test.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,21 @@ const fsMocks = vi.hoisted(() => ({
77
rm: vi.fn(),
88
}));
99

10+
const childProcessMocks = vi.hoisted(() => ({
11+
spawn: vi.fn(() => ({
12+
unref: vi.fn(),
13+
})),
14+
}));
15+
1016
vi.mock('node:fs/promises', () => ({
1117
default: fsMocks,
1218
...fsMocks,
1319
}));
1420

21+
vi.mock('node:child_process', () => ({
22+
spawn: childProcessMocks.spawn,
23+
}));
24+
1525
import { ensureAuthenticated, readStoredAuth, refreshStoredAuth } from './auth.js';
1626
import type { StoredAuth } from './types.js';
1727

@@ -52,6 +62,7 @@ beforeEach(() => {
5262
fsMocks.mkdir.mockResolvedValue(undefined);
5363
fsMocks.rm.mockReset();
5464
fsMocks.rm.mockResolvedValue(undefined);
65+
childProcessMocks.spawn.mockClear();
5566
});
5667

5768
describe('readStoredAuth', () => {
@@ -180,6 +191,57 @@ describe('ensureAuthenticated', () => {
180191
const calledUrl = String(fetchSpy.mock.calls[0][0]);
181192
expect(calledUrl).toContain('origin.example');
182193
});
194+
195+
it('keeps waiting after a stray local callback with an invalid state', async () => {
196+
const logSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined);
197+
const authPromise = ensureAuthenticated('https://example.com/cloud', { force: true });
198+
199+
await vi.waitFor(() => {
200+
expect(logSpy).toHaveBeenCalledWith(expect.stringContaining('Opening browser for cloud login: '));
201+
});
202+
203+
const loginLine = logSpy.mock.calls
204+
.map((call) => String(call[0]))
205+
.find((line) => line.startsWith('Opening browser for cloud login: '));
206+
expect(loginLine).toBeTruthy();
207+
208+
const loginUrl = new URL(String(loginLine).slice('Opening browser for cloud login: '.length));
209+
const callbackUrl = new URL(String(loginUrl.searchParams.get('redirect_uri')));
210+
const state = loginUrl.searchParams.get('state');
211+
expect(state).toBeTruthy();
212+
213+
const strayResponse = await fetch(callbackUrl, { redirect: 'manual' });
214+
expect(strayResponse.status).toBe(400);
215+
await expect(strayResponse.text()).resolves.toContain('Ignored invalid CLI login callback');
216+
217+
const stillWaiting = await Promise.race([
218+
authPromise.then(
219+
() => 'resolved',
220+
() => 'rejected'
221+
),
222+
new Promise<'pending'>((resolve) => setTimeout(() => resolve('pending'), 25)),
223+
]);
224+
expect(stillWaiting).toBe('pending');
225+
226+
callbackUrl.searchParams.set('state', String(state));
227+
callbackUrl.searchParams.set('access_token', 'access-token');
228+
callbackUrl.searchParams.set('refresh_token', 'refresh-token');
229+
callbackUrl.searchParams.set('access_token_expires_at', '2999-01-01T00:00:00.000Z');
230+
callbackUrl.searchParams.set('api_url', 'https://example.com/cloud');
231+
232+
const successResponse = await fetch(callbackUrl, { redirect: 'manual' });
233+
expect(successResponse.status).toBe(302);
234+
235+
await expect(authPromise).resolves.toEqual({
236+
apiUrl: 'https://example.com/cloud',
237+
accessToken: 'access-token',
238+
refreshToken: 'refresh-token',
239+
accessTokenExpiresAt: '2999-01-01T00:00:00.000Z',
240+
});
241+
expect(fsMocks.writeFile).toHaveBeenCalledOnce();
242+
243+
logSpy.mockRestore();
244+
});
183245
});
184246

185247
describe('refreshStoredAuth', () => {

packages/cloud/src/auth.ts

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -161,15 +161,9 @@ async function beginBrowserLogin(apiUrl: string): Promise<StoredAuth> {
161161
// Validate state parameter first (CSRF protection) — this check
162162
// must run unconditionally, before any user-controlled values.
163163
if (returnedState !== state) {
164-
redirectToHostedCliAuthPage(response, apiUrl, {
165-
status: 'error',
166-
detail: 'Invalid state parameter',
167-
});
168-
if (!settled) {
169-
settled = true;
170-
server.close();
171-
reject(new Error('Invalid state parameter in CLI login callback'));
172-
}
164+
response.statusCode = 400;
165+
response.setHeader('content-type', 'text/plain; charset=utf-8');
166+
response.end('Ignored invalid CLI login callback. Return to your terminal to continue login.');
173167
return;
174168
}
175169

0 commit comments

Comments
 (0)