mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
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.
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
import { createHash, randomBytes, randomInt } from 'node:crypto'
|
||||
import { sleep } from '../helpers'
|
||||
import { REDIRECT_STATUSES, redirectEndpoint, SimApiError } from '../http/client'
|
||||
import { USER_AGENT } from '../version'
|
||||
|
||||
/**
|
||||
* The terminal half of the CLI key handoff.
|
||||
@@ -167,7 +168,11 @@ export async function pollForKey(
|
||||
try {
|
||||
response = await fetch(new URL(POLL_PATH, endpoint), {
|
||||
method: 'POST',
|
||||
headers: { 'content-type': 'application/json', accept: 'application/json' },
|
||||
headers: {
|
||||
'content-type': 'application/json',
|
||||
accept: 'application/json',
|
||||
'user-agent': USER_AGENT,
|
||||
},
|
||||
body: JSON.stringify({ request: auth.request, verifier: auth.pollSecret }),
|
||||
signal,
|
||||
redirect: 'manual',
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { CLI_CONTRACT } from '../contract/commands'
|
||||
import { V2_OPERATIONS, type V2OperationName } from '../generated/v2-api'
|
||||
import { USER_AGENT } from '../version'
|
||||
import {
|
||||
formatApiErrorDetails,
|
||||
redirectEndpoint,
|
||||
@@ -313,6 +314,29 @@ describe('non-JSON responses', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('request identity', () => {
|
||||
it('identifies the CLI, its version and its runtime to the API', async () => {
|
||||
// Without a User-Agent a CLI request is indistinguishable from any other
|
||||
// API traffic, so a bug that only reproduces on one version cannot be found
|
||||
// in the server's own logs.
|
||||
const fetchMock = vi.fn().mockResolvedValue(
|
||||
new Response(JSON.stringify({ data: [] }), {
|
||||
status: 200,
|
||||
headers: { 'content-type': 'application/json' },
|
||||
})
|
||||
)
|
||||
vi.stubGlobal('fetch', fetchMock)
|
||||
|
||||
await client().request('/api/v2/workflows')
|
||||
|
||||
const headers = fetchMock.mock.calls[0][1].headers as Record<string, string>
|
||||
expect(headers['user-agent']).toBe(USER_AGENT)
|
||||
expect(USER_AGENT).toMatch(/^sim-cli\/\d+\.\d+\.\d+/)
|
||||
expect(USER_AGENT).toContain(`node/${process.versions.node}`)
|
||||
expect(USER_AGENT).toContain(process.platform)
|
||||
})
|
||||
})
|
||||
|
||||
describe('personal-key-only operations', () => {
|
||||
it('appends the remedy, keyed off the code the API actually nests', async () => {
|
||||
// The envelope this asserts is the one staging returns: `error.code` is the
|
||||
@@ -341,6 +365,34 @@ describe('personal-key-only operations', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('also recognises the principal-kind refusal, whose message is written for a log', async () => {
|
||||
// The same refusal is raised at two layers under two codes. The
|
||||
// principal-kind one answers "Principal kind workspace_api_key cannot
|
||||
// perform operation audit_logs.list" — accurate, and useless to a reader
|
||||
// who has no way to act on it. Recognising only the other code left every
|
||||
// audit-log command stating the problem in server vocabulary with no remedy.
|
||||
vi.stubGlobal(
|
||||
'fetch',
|
||||
vi.fn().mockResolvedValue(
|
||||
new Response(
|
||||
JSON.stringify({
|
||||
error: {
|
||||
code: 'FORBIDDEN',
|
||||
message: 'Principal kind workspace_api_key cannot perform operation audit_logs.list',
|
||||
details: { code: 'PRINCIPAL_KIND_NOT_PERMITTED' },
|
||||
},
|
||||
}),
|
||||
{ status: 403, headers: { 'content-type': 'application/json' } }
|
||||
)
|
||||
)
|
||||
)
|
||||
|
||||
await expect(client().request('/api/v2/audit-logs')).rejects.toMatchObject({
|
||||
message:
|
||||
'Principal kind workspace_api_key cannot perform operation audit_logs.list — this operation needs a personal API key: sim login --profile default',
|
||||
})
|
||||
})
|
||||
|
||||
it('invents no remedy for other forbidden codes', async () => {
|
||||
vi.stubGlobal(
|
||||
'fetch',
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import chalk from 'chalk'
|
||||
import type { ResolvedProfile } from '../config/index'
|
||||
import { USER_AGENT } from '../version'
|
||||
|
||||
/**
|
||||
* 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])
|
||||
/** Markup a JSON endpoint would never answer with — a proxy or landing page. */
|
||||
const MARKUP_PREFIX = /^\s*<(?:!doctype|html|\?xml)/i
|
||||
|
||||
/** The one 403 cause the CLI can turn into an instruction. */
|
||||
const WORKSPACE_KEY_REFUSAL = 'WORKSPACE_KEY_OPERATION_NOT_PERMITTED'
|
||||
/**
|
||||
* The 403 causes the CLI can turn into an instruction.
|
||||
*
|
||||
* Two codes describe the same refusal because they are raised at different
|
||||
* layers: the workspace-key policy answers `WORKSPACE_KEY_OPERATION_NOT_PERMITTED`,
|
||||
* while the principal-kind check answers `PRINCIPAL_KIND_NOT_PERMITTED` with a
|
||||
* message written for a server log ("Principal kind workspace_api_key cannot
|
||||
* perform operation audit_logs.list"). Recognising only the first left the
|
||||
* audit-log commands stating the refusal in vocabulary the reader has no way to
|
||||
* act on, and without the one sentence that resolves it.
|
||||
*/
|
||||
const KEY_SCOPE_REFUSALS = new Set([
|
||||
'WORKSPACE_KEY_OPERATION_NOT_PERMITTED',
|
||||
'PRINCIPAL_KIND_NOT_PERMITTED',
|
||||
])
|
||||
|
||||
/**
|
||||
* Names the response the server actually sent, for a body that is not JSON.
|
||||
@@ -156,8 +170,8 @@ function truncate(value: string, max: number): string {
|
||||
}
|
||||
|
||||
/**
|
||||
* The refusal a workspace-scoped key gets from an operation only a personal key
|
||||
* may perform.
|
||||
* Whether this is the refusal a workspace-scoped key gets from an operation only
|
||||
* a personal key may perform, under either code that expresses it.
|
||||
*
|
||||
* `error.code` is the envelope's status class and is plain `FORBIDDEN` here; the
|
||||
* actionable code rides in `error.details.code`, which is where the v2 error
|
||||
@@ -165,11 +179,12 @@ function truncate(value: string, max: number): string {
|
||||
* code meant the remedy was never appended against the real API. The top level
|
||||
* is still accepted so a server that promotes the code stays covered.
|
||||
*/
|
||||
function namesWorkspaceKeyRefusal(error: SimApiError): boolean {
|
||||
if (error.code === WORKSPACE_KEY_REFUSAL) return true
|
||||
function namesKeyScopeRefusal(error: SimApiError): boolean {
|
||||
if (typeof error.code === 'string' && KEY_SCOPE_REFUSALS.has(error.code)) return true
|
||||
const details = error.details
|
||||
if (!details || typeof details !== 'object') return false
|
||||
return (details as { code?: unknown }).code === WORKSPACE_KEY_REFUSAL
|
||||
const code = (details as { code?: unknown }).code
|
||||
return typeof code === 'string' && KEY_SCOPE_REFUSALS.has(code)
|
||||
}
|
||||
|
||||
interface DetailIssue {
|
||||
@@ -342,6 +357,7 @@ export class SimClient {
|
||||
headers: {
|
||||
...(apiKey ? { 'x-api-key': apiKey } : {}),
|
||||
accept: 'application/json',
|
||||
'user-agent': USER_AGENT,
|
||||
...(hasBody ? { 'content-type': 'application/json' } : {}),
|
||||
...options.headers,
|
||||
},
|
||||
@@ -367,7 +383,7 @@ export class SimClient {
|
||||
if (response.status === 401) {
|
||||
error.message = `${error.message} — run: sim login --profile ${this.profile.name}`
|
||||
}
|
||||
if (namesWorkspaceKeyRefusal(error)) {
|
||||
if (namesKeyScopeRefusal(error)) {
|
||||
error.message = `${error.message} — this operation needs a personal API key: sim login --profile ${this.profile.name}`
|
||||
}
|
||||
throw error
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
import { readFileSync } from 'node:fs'
|
||||
import { Command, Option } from 'commander'
|
||||
import { loginCommand, logoutCommand, profilesCommand, whoamiCommand } from './commands/auth'
|
||||
import { configureCommand } from './commands/configure'
|
||||
@@ -7,6 +6,7 @@ import { attachProtocolCommands } from './commands/protocol/index'
|
||||
import { attachSecretCommands } from './commands/secrets'
|
||||
import { OUTPUT_FORMATS } from './config/index'
|
||||
import { buildGeneratedCommands } from './runtime/build'
|
||||
import { CLI_VERSION } from './version'
|
||||
|
||||
/** Root program description, shared by `--help` and the generated docs. */
|
||||
export const PROGRAM_DESCRIPTION = 'Talk to the Sim API from your terminal'
|
||||
@@ -28,21 +28,6 @@ Examples:
|
||||
$ sim whoami --profile dev
|
||||
`
|
||||
|
||||
function readPackageVersion(): string {
|
||||
const metadata: unknown = JSON.parse(
|
||||
readFileSync(new URL('../package.json', import.meta.url), 'utf8')
|
||||
)
|
||||
if (
|
||||
typeof metadata !== 'object' ||
|
||||
metadata === null ||
|
||||
!('version' in metadata) ||
|
||||
typeof metadata.version !== 'string'
|
||||
) {
|
||||
throw new Error('CLI package metadata is missing a valid version')
|
||||
}
|
||||
return metadata.version
|
||||
}
|
||||
|
||||
/**
|
||||
* Assemble the complete command tree.
|
||||
*
|
||||
@@ -60,7 +45,7 @@ export function buildProgram(options: { version?: boolean } = {}): Command {
|
||||
|
||||
program.name('sim').description(PROGRAM_DESCRIPTION)
|
||||
|
||||
if (options.version !== false) program.version(readPackageVersion())
|
||||
if (options.version !== false) program.version(CLI_VERSION)
|
||||
|
||||
program
|
||||
.option('-P, --profile <name>', 'Profile to use (env: SIM_PROFILE)')
|
||||
|
||||
@@ -0,0 +1,41 @@
|
||||
import { readFileSync } from 'node:fs'
|
||||
|
||||
/**
|
||||
* The published package version, and the `User-Agent` built from it.
|
||||
*
|
||||
* Its own module because both the command tree and the HTTP client need the
|
||||
* version, and the client cannot reach `program.ts` — `program` builds the
|
||||
* commands, which reach the client, so importing it back would close a cycle.
|
||||
*
|
||||
* Read from `package.json` rather than inlined so a release cannot ship a
|
||||
* version string that disagrees with the package it came from. The bundle keeps
|
||||
* `dist/index.js` one directory below the manifest, and npm always publishes the
|
||||
* manifest, so the relative path holds for an installed package as well as a
|
||||
* local build.
|
||||
*/
|
||||
function readPackageVersion(): string {
|
||||
const metadata: unknown = JSON.parse(
|
||||
readFileSync(new URL('../package.json', import.meta.url), 'utf8')
|
||||
)
|
||||
if (
|
||||
typeof metadata !== 'object' ||
|
||||
metadata === null ||
|
||||
!('version' in metadata) ||
|
||||
typeof metadata.version !== 'string'
|
||||
) {
|
||||
throw new Error('CLI package metadata is missing a valid version')
|
||||
}
|
||||
return metadata.version
|
||||
}
|
||||
|
||||
export const CLI_VERSION = readPackageVersion()
|
||||
|
||||
/**
|
||||
* Identifies the CLI to the API, the way every other terminal client does.
|
||||
*
|
||||
* Without it a CLI request is indistinguishable from any other API traffic, so
|
||||
* a bug that only reproduces on one CLI version cannot be found in the server's
|
||||
* own logs. The runtime and platform ride along for the same reason: they are
|
||||
* the first things asked about a transport failure that only some users see.
|
||||
*/
|
||||
export const USER_AGENT = `sim-cli/${CLI_VERSION} node/${process.versions.node} (${process.platform}; ${process.arch})`
|
||||
Reference in New Issue
Block a user