Skip to content

Commit 05ee6b9

Browse files
committed
feat(http): validate the /me and usage response shapes at runtime
Extends the #266 validation layer to the account surfaces. `GET /me` is read by three commands through the generic `client.get`, all of which blind-cast the body: - `auth status`/`whoami` renders `m.scopes.join(', ')` and computes missing scopes via `m.scopes.includes(...)` with no guard, so a `/me` body without `scopes` crashed with a raw `TypeError: Cannot read properties of undefined (reading 'join')` (exit 1). Under `--output json` it was worse: the renderer never ran, so the CLI printed the partial identity and exited 0, and an agent reading `scopes` got undefined. - `usage` feeds `credits` / `creditsPerRun` into `Math.floor(credits / creditsPerRun)`, so a string balance reached the pre-flight arithmetic unchecked. - `doctor`'s connectivity probe reads a fully-optional projection whose local interface had already drifted from the stubbed schema (`v3Enabled` existed on the command side only). Adds `ME_RESPONSE_SCHEMA` and `USAGE_RESPONSE_SCHEMA`, wires the previously-unwired `ME_IDENTITY_SCHEMA` into `doctor`, and aliases doctor's `MeIdentity` to the schema's wire type so the two cannot drift again. Drift now surfaces as the standard typed INTERNAL envelope naming the mismatched field paths. Follows the #266 policy unchanged: every object is `looseObject` so additive server fields pass through and reach `--output json` untouched; `env` is validated as an open string via `openWireLiteral` so a new deployment tier cannot hard-fail; only fields that every `/me` fixture in the suite supplies (`userId`, `keyId`, `scopes`, `env`) are required, and the genuinely absent-safe ones (`email`, `displayName`, `v3Enabled`, `credits`, `subPlan`, `creditsPerRun`) stay optional with no default. Refs #277
1 parent fe07bc9 commit 05ee6b9

7 files changed

Lines changed: 247 additions & 21 deletions

File tree

src/commands/auth.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -850,6 +850,41 @@ describe('runWhoami', () => {
850850
expect(printed).toEqual(sampleMe);
851851
});
852852

853+
// Issue #277: `/me` is validated against ME_RESPONSE_SCHEMA at the client
854+
// boundary, so wire drift becomes a typed envelope instead of a crash.
855+
it('turns a /me body without scopes into a typed INTERNAL envelope, not a raw TypeError', async () => {
856+
writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath });
857+
const { deps } = makeCapture();
858+
const withoutScopes: Record<string, unknown> = { ...sampleMe };
859+
delete withoutScopes.scopes;
860+
const drifted = new Response(JSON.stringify(withoutScopes), {
861+
status: 200,
862+
headers: { 'content-type': 'application/json' },
863+
});
864+
const rejection = await runWhoami(
865+
{ profile: 'default', output: 'text', debug: false },
866+
{ ...deps, env: {}, credentialsPath, fetchImpl: makeFetch(drifted) },
867+
).catch((error: unknown) => error);
868+
// Before: `m.scopes.join(', ')` threw `TypeError: ... not a function`.
869+
expect(rejection).toBeInstanceOf(ApiError);
870+
expect(rejection).toMatchObject({ code: 'INTERNAL' });
871+
expect(JSON.stringify((rejection as ApiError).getDetail('issues'))).toContain('scopes');
872+
});
873+
874+
it('passes an additive /me field straight through to --output json', async () => {
875+
writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath });
876+
const { capture, deps } = makeCapture();
877+
const additive = new Response(JSON.stringify({ ...sampleMe, orgName: 'Acme' }), {
878+
status: 200,
879+
headers: { 'content-type': 'application/json' },
880+
});
881+
await runWhoami(
882+
{ profile: 'default', output: 'json', debug: false },
883+
{ ...deps, env: {}, credentialsPath, fetchImpl: makeFetch(additive) },
884+
);
885+
expect(JSON.parse(capture.stdout.join('')).orgName).toBe('Acme');
886+
});
887+
853888
it('renders routing: v3 and the gap advisory when v3Enabled is true', async () => {
854889
writeProfile('default', { apiKey: 'sk' }, { path: credentialsPath });
855890
const { capture, deps } = makeCapture();

src/commands/auth.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { emitDeprecationNotice } from '../lib/deprecate.js';
2222
import type { OutputMode } from '../lib/output.js';
2323
import { GLOBAL_OPTS_HINT, Output, resolveOutputMode } from '../lib/output.js';
2424
import { promptSecret } from '../lib/prompt.js';
25+
import { ME_RESPONSE_SCHEMA } from '../lib/response-schemas.js';
2526
import { emitV3RoutingAdvisory, routingLabel } from '../lib/v3-advisory.js';
2627

2728
export interface MeResponse {
@@ -261,7 +262,7 @@ export async function runWhoami(opts: CommonOptions, deps: AuthDeps = {}): Promi
261262
stderr: deps.stderr,
262263
});
263264

264-
const me = await client.get<MeResponse>('/me');
265+
const me = await client.get<MeResponse>('/me', { schema: ME_RESPONSE_SCHEMA });
265266
out.print(me, data => {
266267
const m = data as MeResponse;
267268
const lines = [

src/commands/doctor.ts

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ import { loadConfig } from '../lib/config.js';
2525
import { ApiError, CLIError, localValidationError } from '../lib/errors.js';
2626
import type { FetchImpl } from '../lib/http.js';
2727
import { GLOBAL_OPTS_HINT, Output, resolveOutputMode, type OutputMode } from '../lib/output.js';
28+
import type { MeIdentityWire } from '../lib/response-schemas.js';
29+
import { ME_IDENTITY_SCHEMA } from '../lib/response-schemas.js';
2830
import { isVerifySkillInstalled } from '../lib/skill-nudge.js';
2931
import { emitV3RoutingAdvisory, routingLabel } from '../lib/v3-advisory.js';
3032
import { VERSION } from '../version.js';
@@ -46,12 +48,13 @@ export interface DoctorReport {
4648
warnings: number;
4749
}
4850

49-
/** Minimal projection of `GET /me` we read for the connectivity detail. */
50-
interface MeIdentity {
51-
userId?: string;
52-
keyId?: string;
53-
v3Enabled?: boolean;
54-
}
51+
/**
52+
* Minimal projection of `GET /me` we read for the connectivity detail.
53+
*
54+
* Aliased to the schema's wire type so the interface and
55+
* {@link ME_IDENTITY_SCHEMA} cannot drift apart (issue #277).
56+
*/
57+
type MeIdentity = MeIdentityWire;
5558

5659
export interface DoctorDeps {
5760
env?: NodeJS.ProcessEnv;
@@ -213,7 +216,7 @@ async function checkConnectivity(
213216
fetchImpl: deps.fetchImpl,
214217
stderr: deps.stderr,
215218
});
216-
const me = await client.get<MeIdentity>('/me');
219+
const me = await client.get<MeIdentity>('/me', { schema: ME_IDENTITY_SCHEMA });
217220
const who = me.userId ? ` (userId ${me.userId})` : '';
218221
return {
219222
check: { name, status: 'ok', detail: `reached GET /me, API key accepted${who}` },

src/commands/usage.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,23 @@ describe('runUsage — real path without credits (current backend)', () => {
115115
expect(stderr).toContain('testsprite.com');
116116
});
117117

118+
// Issue #277: the usage projection is validated at the client boundary, so a
119+
// string balance surfaces as a typed envelope instead of silently reaching
120+
// the `Math.floor(credits / creditsPerRun)` pre-flight arithmetic.
121+
it('rejects a non-numeric credit balance with a typed INTERNAL envelope', async () => {
122+
writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath });
123+
const { deps } = makeCapture();
124+
const rejection = await runUsage(
125+
{ profile: 'default', output: 'text', debug: false },
126+
{
127+
...deps,
128+
credentialsPath,
129+
fetchImpl: makeFetch({ ...meWithoutCredits, credits: '100', creditsPerRun: 2 }),
130+
},
131+
).catch((error: unknown) => error);
132+
expect(rejection).toMatchObject({ code: 'INTERNAL' });
133+
});
134+
118135
it('text output includes identity fields even without credits', async () => {
119136
writeProfile('default', { apiKey: 'sk-abc' }, { path: credentialsPath });
120137
const { capture, deps } = makeCapture();

src/commands/usage.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import { loadConfig } from '../lib/config.js';
2424
import { resolvePortalBase } from '../lib/facade.js';
2525
import type { FetchImpl } from '../lib/http.js';
2626
import { GLOBAL_OPTS_HINT, Output, resolveOutputMode, type OutputMode } from '../lib/output.js';
27+
import { USAGE_RESPONSE_SCHEMA } from '../lib/response-schemas.js';
2728

2829
/**
2930
* Usage/balance response from `/me` (when the backend supplies it) or a future
@@ -117,7 +118,7 @@ export async function runUsage(opts: CommonOptions, deps: UsageDeps = {}): Promi
117118
// /me is the only available source of credits/plan today.
118119
// When the backend adds credits/subPlan to MeResponse (or adds /usage),
119120
// this single get call is sufficient — no code change needed in the CLI.
120-
const me = await client.get<UsageResponse>('/me');
121+
const me = await client.get<UsageResponse>('/me', { schema: USAGE_RESPONSE_SCHEMA });
121122

122123
out.print(me, data => renderUsage(data as UsageResponse, portalBase));
123124

src/lib/response-schemas.test.ts

Lines changed: 101 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,13 @@
77
import { describe, expect, it } from 'vitest';
88
import * as v from 'valibot';
99
import { HttpClient } from './http.js';
10-
import { RUN_RESPONSE_SCHEMA, TRIGGER_RUN_RESPONSE_SCHEMA } from './response-schemas.js';
10+
import {
11+
ME_IDENTITY_SCHEMA,
12+
ME_RESPONSE_SCHEMA,
13+
RUN_RESPONSE_SCHEMA,
14+
TRIGGER_RUN_RESPONSE_SCHEMA,
15+
USAGE_RESPONSE_SCHEMA,
16+
} from './response-schemas.js';
1117

1218
const VALID_RUN = {
1319
runId: 'run_1',
@@ -102,3 +108,97 @@ describe('TRIGGER_RUN_RESPONSE_SCHEMA', () => {
102108
expect(parsed.success).toBe(true);
103109
});
104110
});
111+
112+
// ---------------------------------------------------------------------------
113+
// Account surfaces (issue #277): GET /me and its usage projection.
114+
// ---------------------------------------------------------------------------
115+
116+
const VALID_ME = {
117+
userId: 'u_1',
118+
keyId: 'k_1',
119+
scopes: ['read:projects', 'read:tests'],
120+
env: 'development',
121+
};
122+
123+
describe('ME_RESPONSE_SCHEMA', () => {
124+
it('accepts the minimal /me body and preserves unknown extra keys', () => {
125+
const parsed = v.safeParse(ME_RESPONSE_SCHEMA, { ...VALID_ME, plan: 'Pro' });
126+
expect(parsed.success).toBe(true);
127+
if (parsed.success) {
128+
expect((parsed.output as { plan?: string }).plan).toBe('Pro');
129+
}
130+
});
131+
132+
it('accepts an unknown env value (a new deployment tier must not hard-fail)', () => {
133+
expect(v.safeParse(ME_RESPONSE_SCHEMA, { ...VALID_ME, env: 'sandbox' }).success).toBe(true);
134+
});
135+
136+
it('leaves the absent-safe identity fields absent rather than defaulting them', () => {
137+
const parsed = v.safeParse(ME_RESPONSE_SCHEMA, VALID_ME);
138+
expect(parsed.success).toBe(true);
139+
if (parsed.success) {
140+
expect('email' in parsed.output).toBe(false);
141+
expect('displayName' in parsed.output).toBe(false);
142+
expect('v3Enabled' in parsed.output).toBe(false);
143+
}
144+
});
145+
146+
it('rejects a /me body without scopes, naming the path', () => {
147+
const withoutScopes: Record<string, unknown> = { ...VALID_ME };
148+
delete withoutScopes.scopes;
149+
const parsed = v.safeParse(ME_RESPONSE_SCHEMA, withoutScopes);
150+
expect(parsed.success).toBe(false);
151+
if (!parsed.success) {
152+
expect(parsed.issues.some(issue => v.getDotPath(issue) === 'scopes')).toBe(true);
153+
}
154+
});
155+
});
156+
157+
describe('ME_IDENTITY_SCHEMA', () => {
158+
it("accepts doctor's partial identity projection (connectivity must not fail on it)", () => {
159+
expect(v.safeParse(ME_IDENTITY_SCHEMA, { userId: 'u-doc', keyId: 'k-doc' }).success).toBe(true);
160+
});
161+
162+
it('carries v3Enabled through so the routing advisory still fires', () => {
163+
const parsed = v.safeParse(ME_IDENTITY_SCHEMA, { ...VALID_ME, v3Enabled: true });
164+
expect(parsed.success).toBe(true);
165+
if (parsed.success) {
166+
expect(parsed.output.v3Enabled).toBe(true);
167+
}
168+
});
169+
170+
it('rejects a wrongly-typed identity field', () => {
171+
expect(v.safeParse(ME_IDENTITY_SCHEMA, { userId: 42 }).success).toBe(false);
172+
});
173+
});
174+
175+
describe('USAGE_RESPONSE_SCHEMA', () => {
176+
it("accepts today's /me body, which carries no credits fields at all", () => {
177+
const parsed = v.safeParse(USAGE_RESPONSE_SCHEMA, VALID_ME);
178+
expect(parsed.success).toBe(true);
179+
if (parsed.success) {
180+
expect(parsed.output.credits).toBeUndefined();
181+
// `scopes` is not part of the usage projection but must survive as an
182+
// unknown extra key so `--output json` stays byte-faithful.
183+
expect((parsed.output as { scopes?: string[] }).scopes).toEqual(VALID_ME.scopes);
184+
}
185+
});
186+
187+
it('accepts the future body with credits, plan and per-run cost', () => {
188+
const parsed = v.safeParse(USAGE_RESPONSE_SCHEMA, {
189+
...VALID_ME,
190+
credits: 100,
191+
subPlan: 'Standard',
192+
creditsPerRun: 2,
193+
});
194+
expect(parsed.success).toBe(true);
195+
});
196+
197+
it('rejects a non-numeric credit balance instead of rendering NaN math', () => {
198+
const parsed = v.safeParse(USAGE_RESPONSE_SCHEMA, { ...VALID_ME, credits: '100' });
199+
expect(parsed.success).toBe(false);
200+
if (!parsed.success) {
201+
expect(parsed.issues.some(issue => v.getDotPath(issue) === 'credits')).toBe(true);
202+
}
203+
});
204+
});

src/lib/response-schemas.ts

Lines changed: 80 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
/**
2-
* Valibot schemas for the run-path wire shapes (issue #102).
2+
* Valibot schemas for the API wire shapes (issue #102, extended by #277).
33
*
44
* `requestWithMeta` used to return `(await response.json()) as T` with zero
55
* runtime validation, so a drifted or partial server response surfaced as
66
* `undefined` output or an opaque TypeError deep inside a command. These
77
* schemas are wired (opt-in, via `RequestOptions.schema`) into the typed
8-
* HttpClient helpers only: `triggerRun`, `triggerRunWithMeta`, `triggerRerun`,
9-
* `triggerBatchRerun`, `triggerBatchRunFresh`, `getRun`, `listTestRuns`.
10-
* The generic `get`/`post`/`put`/`patch`/`delete` paths stay schema-free.
8+
* HttpClient helpers `triggerRun`, `triggerRunWithMeta`, `triggerRerun`,
9+
* `triggerBatchRerun`, `triggerBatchRunFresh`, `getRun`, `listTestRuns`, and
10+
* — for the account surfaces below — at the `client.get` call sites that own
11+
* a `/me` or usage read. Every other generic `get`/`post`/`put`/`patch`/
12+
* `delete` caller stays schema-free and opt-in.
1113
*
1214
* Resilience rules (additive server changes must never hard-fail the CLI):
1315
*
@@ -32,6 +34,11 @@
3234
* interface it mirrors, so schema/interface drift fails `tsc` in this file.
3335
*/
3436
import * as v from 'valibot';
37+
// Type-only imports: erased at compile time, so pulling a command's wire
38+
// interface into `lib/` adds no runtime edge (same pattern as `bundle.ts`,
39+
// which type-imports `CliTestStep` from `commands/test.ts`).
40+
import type { MeResponse } from '../commands/auth.js';
41+
import type { UsageResponse } from '../commands/usage.js';
3542
import type {
3643
BatchRerunResponse,
3744
BatchRunFreshResponse,
@@ -44,6 +51,9 @@ import type {
4451
TriggerRunResponse,
4552
} from './runs.types.js';
4653

54+
/** Deployment environment the bound key belongs to; open on the wire (rule 2). */
55+
type AccountEnv = 'development' | 'staging' | 'production';
56+
4757
/**
4858
* Compile-time literal union, runtime open string.
4959
*
@@ -251,21 +261,80 @@ export const LIST_RUNS_RESPONSE_SCHEMA: v.GenericSchema<unknown, ListRunsRespons
251261
// ---------------------------------------------------------------------------
252262

253263
/**
254-
* Minimal `/me` identity core shared by its consumers. `doctor` reads a
255-
* two-field optional projection (`MeIdentity` in commands/doctor.ts) while
256-
* `auth whoami` reads the full `MeResponse` (commands/auth.ts); this schema
257-
* validates the common identity core so it can guard either caller, and
258-
* `looseObject` lets the full projection (scopes, env, email, ...) pass
259-
* through untouched. Not wired into any typed helper yet: `/me` callers use
260-
* the generic `get`, which stays schema-free in this change.
264+
* Minimal `/me` identity core, as read by `doctor`'s connectivity check.
265+
*
266+
* `doctor` deliberately treats every field as optional: the check only needs
267+
* "the key was accepted", and it decorates the detail line with the userId
268+
* *when present* (`me.userId ? ...`). Fixture evidence for keeping it fully
269+
* optional rather than reusing {@link ME_RESPONSE_SCHEMA}: `OK_ME` in
270+
* `commands/doctor.test.ts` is `{ userId, keyId }` with no `scopes`/`env`, and
271+
* a connectivity probe must not fail on a partial identity projection.
272+
*
273+
* `commands/doctor.ts` aliases its `MeIdentity` to this type so the two cannot
274+
* drift (they already had: `v3Enabled` existed on the command side only).
261275
*/
262276
export interface MeIdentityWire {
263277
userId?: string;
264278
keyId?: string;
279+
/** Authoritative per-user V3 routing bit; older backends omit it. */
280+
v3Enabled?: boolean;
265281
}
266282

267283
/** Mirrors `MeIdentity` (commands/doctor.ts): `GET /api/cli/v1/me` core. */
268284
export const ME_IDENTITY_SCHEMA: v.GenericSchema<unknown, MeIdentityWire> = v.looseObject({
269285
userId: v.optional(v.string()),
270286
keyId: v.optional(v.string()),
287+
v3Enabled: v.optional(v.boolean()),
288+
});
289+
290+
/**
291+
* Mirrors `MeResponse` (commands/auth.ts): the full `GET /me` projection read
292+
* by `auth whoami` (and, through it, `init`).
293+
*
294+
* `scopes` is required and array-typed on purpose — this is the shape drift
295+
* that actually bites today. `runWhoami` renders `m.scopes.join(', ')` and
296+
* computes `missingScopes` via `m.scopes.includes(...)` with no guard, so a
297+
* `/me` body without `scopes` crashes with a raw `TypeError` (exit 1) instead
298+
* of a typed envelope. Every `/me` fixture in the suite supplies it
299+
* (`auth.test.ts`, `init.test.ts`, `usage.test.ts`, `cli.subprocess.test.ts`,
300+
* `test/mock-backend/fixtures.ts`), so requiring it matches observed wire
301+
* reality; `email` / `displayName` / `v3Enabled` are the genuinely absent-safe
302+
* ones and stay `v.optional` with no default (rule 3, optional branch).
303+
*
304+
* `init` calls this through `runWhoami` inside a try/catch that falls back to
305+
* a placeholder identity, so a drifted `/me` degrades the setup summary
306+
* instead of failing the whole `init`.
307+
*/
308+
export const ME_RESPONSE_SCHEMA: v.GenericSchema<unknown, MeResponse> = v.looseObject({
309+
userId: v.string(),
310+
keyId: v.string(),
311+
scopes: v.array(v.string()),
312+
env: openWireLiteral<AccountEnv>(),
313+
email: v.optional(v.string()),
314+
displayName: v.optional(v.string()),
315+
v3Enabled: v.optional(v.boolean()),
316+
});
317+
318+
// ---------------------------------------------------------------------------
319+
// GET /me (usage projection)
320+
// ---------------------------------------------------------------------------
321+
322+
/**
323+
* Mirrors `UsageResponse` (commands/usage.ts): the credits/plan projection the
324+
* `usage` command reads off the same `GET /me` body.
325+
*
326+
* `renderUsage` prints `userId`/`keyId`/`env` unconditionally as its "identity
327+
* block", so those three are required; `credits`, `subPlan` and
328+
* `creditsPerRun` are forward-compat fields the backend does not send today
329+
* (see the BACKEND FOLLOW-UP note in usage.ts) and every renderer branch is
330+
* gated on `!== undefined`, so they stay optional with no default. `scopes`
331+
* rides along as an unknown extra key and is preserved by `looseObject`.
332+
*/
333+
export const USAGE_RESPONSE_SCHEMA: v.GenericSchema<unknown, UsageResponse> = v.looseObject({
334+
userId: v.string(),
335+
keyId: v.string(),
336+
env: openWireLiteral<AccountEnv>(),
337+
credits: v.optional(v.number()),
338+
subPlan: v.optional(v.string()),
339+
creditsPerRun: v.optional(v.number()),
271340
});

0 commit comments

Comments
 (0)