Skip to content

Commit 5c37778

Browse files
authored
fix(cli): identify the CLI to the API and name the remedy on every key refusal (#6792)
Sends a `User-Agent` on both request paths. Without one a CLI request is indistinguishable from any other API traffic, so a bug that reproduces on a single CLI version cannot be found in the server's own logs; the runtime and platform ride along because they are the first things asked about a transport failure only some users hit. The version moves into its own module so the HTTP client can read it. It could not import `program.ts` — program builds the commands, which reach the client — and duplicating the manifest read would let the two disagree. Also recognises `PRINCIPAL_KIND_NOT_PERMITTED`. The same refusal is raised at two layers under two codes, and only the workspace-key one was matched, so the audit-log commands reported "Principal kind workspace_api_key cannot perform operation audit_logs.list" — accurate, written for a server log, and missing the one sentence that tells the reader a personal key resolves it.
1 parent 38075ad commit 5c37778

5 files changed

Lines changed: 125 additions & 26 deletions

File tree

packages/sim-cli/src/auth/device-flow.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { createHash, randomBytes, randomInt } from 'node:crypto'
22
import { sleep } from '../helpers'
33
import { REDIRECT_STATUSES, redirectEndpoint, SimApiError } from '../http/client'
4+
import { USER_AGENT } from '../version'
45

56
/**
67
* The terminal half of the CLI key handoff.
@@ -167,7 +168,11 @@ export async function pollForKey(
167168
try {
168169
response = await fetch(new URL(POLL_PATH, endpoint), {
169170
method: 'POST',
170-
headers: { 'content-type': 'application/json', accept: 'application/json' },
171+
headers: {
172+
'content-type': 'application/json',
173+
accept: 'application/json',
174+
'user-agent': USER_AGENT,
175+
},
171176
body: JSON.stringify({ request: auth.request, verifier: auth.pollSecret }),
172177
signal,
173178
redirect: 'manual',

packages/sim-cli/src/http/client.test.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { afterEach, describe, expect, it, vi } from 'vitest'
22
import { CLI_CONTRACT } from '../contract/commands'
33
import { V2_OPERATIONS, type V2OperationName } from '../generated/v2-api'
4+
import { USER_AGENT } from '../version'
45
import {
56
formatApiErrorDetails,
67
redirectEndpoint,
@@ -313,6 +314,29 @@ describe('non-JSON responses', () => {
313314
})
314315
})
315316

317+
describe('request identity', () => {
318+
it('identifies the CLI, its version and its runtime to the API', async () => {
319+
// Without a User-Agent a CLI request is indistinguishable from any other
320+
// API traffic, so a bug that only reproduces on one version cannot be found
321+
// in the server's own logs.
322+
const fetchMock = vi.fn().mockResolvedValue(
323+
new Response(JSON.stringify({ data: [] }), {
324+
status: 200,
325+
headers: { 'content-type': 'application/json' },
326+
})
327+
)
328+
vi.stubGlobal('fetch', fetchMock)
329+
330+
await client().request('/api/v2/workflows')
331+
332+
const headers = fetchMock.mock.calls[0][1].headers as Record<string, string>
333+
expect(headers['user-agent']).toBe(USER_AGENT)
334+
expect(USER_AGENT).toMatch(/^sim-cli\/\d+\.\d+\.\d+/)
335+
expect(USER_AGENT).toContain(`node/${process.versions.node}`)
336+
expect(USER_AGENT).toContain(process.platform)
337+
})
338+
})
339+
316340
describe('personal-key-only operations', () => {
317341
it('appends the remedy, keyed off the code the API actually nests', async () => {
318342
// The envelope this asserts is the one staging returns: `error.code` is the
@@ -341,6 +365,34 @@ describe('personal-key-only operations', () => {
341365
})
342366
})
343367

368+
it('also recognises the principal-kind refusal, whose message is written for a log', async () => {
369+
// The same refusal is raised at two layers under two codes. The
370+
// principal-kind one answers "Principal kind workspace_api_key cannot
371+
// perform operation audit_logs.list" — accurate, and useless to a reader
372+
// who has no way to act on it. Recognising only the other code left every
373+
// audit-log command stating the problem in server vocabulary with no remedy.
374+
vi.stubGlobal(
375+
'fetch',
376+
vi.fn().mockResolvedValue(
377+
new Response(
378+
JSON.stringify({
379+
error: {
380+
code: 'FORBIDDEN',
381+
message: 'Principal kind workspace_api_key cannot perform operation audit_logs.list',
382+
details: { code: 'PRINCIPAL_KIND_NOT_PERMITTED' },
383+
},
384+
}),
385+
{ status: 403, headers: { 'content-type': 'application/json' } }
386+
)
387+
)
388+
)
389+
390+
await expect(client().request('/api/v2/audit-logs')).rejects.toMatchObject({
391+
message:
392+
'Principal kind workspace_api_key cannot perform operation audit_logs.list — this operation needs a personal API key: sim login --profile default',
393+
})
394+
})
395+
344396
it('invents no remedy for other forbidden codes', async () => {
345397
vi.stubGlobal(
346398
'fetch',

packages/sim-cli/src/http/client.ts

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import chalk from 'chalk'
22
import type { ResolvedProfile } from '../config/index'
3+
import { USER_AGENT } from '../version'
34

45
/**
56
* A failure the CLI can explain. Anything thrown as a `SimApiError` is printed
@@ -75,8 +76,21 @@ export const REDIRECT_STATUSES = new Set([301, 302, 303, 307, 308])
7576
/** Markup a JSON endpoint would never answer with — a proxy or landing page. */
7677
const MARKUP_PREFIX = /^\s*<(?:!doctype|html|\?xml)/i
7778

78-
/** The one 403 cause the CLI can turn into an instruction. */
79-
const WORKSPACE_KEY_REFUSAL = 'WORKSPACE_KEY_OPERATION_NOT_PERMITTED'
79+
/**
80+
* The 403 causes the CLI can turn into an instruction.
81+
*
82+
* Two codes describe the same refusal because they are raised at different
83+
* layers: the workspace-key policy answers `WORKSPACE_KEY_OPERATION_NOT_PERMITTED`,
84+
* while the principal-kind check answers `PRINCIPAL_KIND_NOT_PERMITTED` with a
85+
* message written for a server log ("Principal kind workspace_api_key cannot
86+
* perform operation audit_logs.list"). Recognising only the first left the
87+
* audit-log commands stating the refusal in vocabulary the reader has no way to
88+
* act on, and without the one sentence that resolves it.
89+
*/
90+
const KEY_SCOPE_REFUSALS = new Set([
91+
'WORKSPACE_KEY_OPERATION_NOT_PERMITTED',
92+
'PRINCIPAL_KIND_NOT_PERMITTED',
93+
])
8094

8195
/**
8296
* Names the response the server actually sent, for a body that is not JSON.
@@ -156,20 +170,21 @@ function truncate(value: string, max: number): string {
156170
}
157171

158172
/**
159-
* The refusal a workspace-scoped key gets from an operation only a personal key
160-
* may perform.
173+
* Whether this is the refusal a workspace-scoped key gets from an operation only
174+
* a personal key may perform, under either code that expresses it.
161175
*
162176
* `error.code` is the envelope's status class and is plain `FORBIDDEN` here; the
163177
* actionable code rides in `error.details.code`, which is where the v2 error
164178
* projection puts a refusal that names its cause. Reading only the top-level
165179
* code meant the remedy was never appended against the real API. The top level
166180
* is still accepted so a server that promotes the code stays covered.
167181
*/
168-
function namesWorkspaceKeyRefusal(error: SimApiError): boolean {
169-
if (error.code === WORKSPACE_KEY_REFUSAL) return true
182+
function namesKeyScopeRefusal(error: SimApiError): boolean {
183+
if (typeof error.code === 'string' && KEY_SCOPE_REFUSALS.has(error.code)) return true
170184
const details = error.details
171185
if (!details || typeof details !== 'object') return false
172-
return (details as { code?: unknown }).code === WORKSPACE_KEY_REFUSAL
186+
const code = (details as { code?: unknown }).code
187+
return typeof code === 'string' && KEY_SCOPE_REFUSALS.has(code)
173188
}
174189

175190
interface DetailIssue {
@@ -342,6 +357,7 @@ export class SimClient {
342357
headers: {
343358
...(apiKey ? { 'x-api-key': apiKey } : {}),
344359
accept: 'application/json',
360+
'user-agent': USER_AGENT,
345361
...(hasBody ? { 'content-type': 'application/json' } : {}),
346362
...options.headers,
347363
},
@@ -367,7 +383,7 @@ export class SimClient {
367383
if (response.status === 401) {
368384
error.message = `${error.message} — run: sim login --profile ${this.profile.name}`
369385
}
370-
if (namesWorkspaceKeyRefusal(error)) {
386+
if (namesKeyScopeRefusal(error)) {
371387
error.message = `${error.message} — this operation needs a personal API key: sim login --profile ${this.profile.name}`
372388
}
373389
throw error

packages/sim-cli/src/program.ts

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { readFileSync } from 'node:fs'
21
import { Command, Option } from 'commander'
32
import { loginCommand, logoutCommand, profilesCommand, whoamiCommand } from './commands/auth'
43
import { configureCommand } from './commands/configure'
@@ -7,6 +6,7 @@ import { attachProtocolCommands } from './commands/protocol/index'
76
import { attachSecretCommands } from './commands/secrets'
87
import { OUTPUT_FORMATS } from './config/index'
98
import { buildGeneratedCommands } from './runtime/build'
9+
import { CLI_VERSION } from './version'
1010

1111
/** Root program description, shared by `--help` and the generated docs. */
1212
export const PROGRAM_DESCRIPTION = 'Talk to the Sim API from your terminal'
@@ -28,21 +28,6 @@ Examples:
2828
$ sim whoami --profile dev
2929
`
3030

31-
function readPackageVersion(): string {
32-
const metadata: unknown = JSON.parse(
33-
readFileSync(new URL('../package.json', import.meta.url), 'utf8')
34-
)
35-
if (
36-
typeof metadata !== 'object' ||
37-
metadata === null ||
38-
!('version' in metadata) ||
39-
typeof metadata.version !== 'string'
40-
) {
41-
throw new Error('CLI package metadata is missing a valid version')
42-
}
43-
return metadata.version
44-
}
45-
4631
/**
4732
* Assemble the complete command tree.
4833
*
@@ -60,7 +45,7 @@ export function buildProgram(options: { version?: boolean } = {}): Command {
6045

6146
program.name('sim').description(PROGRAM_DESCRIPTION)
6247

63-
if (options.version !== false) program.version(readPackageVersion())
48+
if (options.version !== false) program.version(CLI_VERSION)
6449

6550
program
6651
.option('-P, --profile <name>', 'Profile to use (env: SIM_PROFILE)')

packages/sim-cli/src/version.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import { readFileSync } from 'node:fs'
2+
3+
/**
4+
* The published package version, and the `User-Agent` built from it.
5+
*
6+
* Its own module because both the command tree and the HTTP client need the
7+
* version, and the client cannot reach `program.ts` — `program` builds the
8+
* commands, which reach the client, so importing it back would close a cycle.
9+
*
10+
* Read from `package.json` rather than inlined so a release cannot ship a
11+
* version string that disagrees with the package it came from. The bundle keeps
12+
* `dist/index.js` one directory below the manifest, and npm always publishes the
13+
* manifest, so the relative path holds for an installed package as well as a
14+
* local build.
15+
*/
16+
function readPackageVersion(): string {
17+
const metadata: unknown = JSON.parse(
18+
readFileSync(new URL('../package.json', import.meta.url), 'utf8')
19+
)
20+
if (
21+
typeof metadata !== 'object' ||
22+
metadata === null ||
23+
!('version' in metadata) ||
24+
typeof metadata.version !== 'string'
25+
) {
26+
throw new Error('CLI package metadata is missing a valid version')
27+
}
28+
return metadata.version
29+
}
30+
31+
export const CLI_VERSION = readPackageVersion()
32+
33+
/**
34+
* Identifies the CLI to the API, the way every other terminal client does.
35+
*
36+
* Without it a CLI request is indistinguishable from any other API traffic, so
37+
* a bug that only reproduces on one CLI version cannot be found in the server's
38+
* own logs. The runtime and platform ride along for the same reason: they are
39+
* the first things asked about a transport failure that only some users see.
40+
*/
41+
export const USER_AGENT = `sim-cli/${CLI_VERSION} node/${process.versions.node} (${process.platform}; ${process.arch})`

0 commit comments

Comments
 (0)