Skip to content

Commit 20fa499

Browse files
committed
fix(api): error on array body + fields, and --data + field flag conflicts
Addresses Seer review comments: - HIGH: buildFromFields now throws ValidationError when a JSON array body is auto-detected alongside -F/-f field flags, instead of silently discarding the array. Arrays can't be meaningfully merged with key=value fields — tell the user explicitly. - MEDIUM: func() now throws ValidationError when --data is combined with --field or --raw-field. Previously the field flags were silently ignored. These flags are mutually exclusive — --data owns the full body. Add E2E tests for all three mutual-exclusivity error paths: --data + --input, --data + --field, --data + --raw-field (-d + -f). Add unit test for array body + fields error in buildFromFields.
1 parent 523fdf5 commit 20fa499

File tree

4 files changed

+113
-8
lines changed

4 files changed

+113
-8
lines changed

AGENTS.md

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -648,8 +648,6 @@ mock.module("./some-module", () => ({
648648
649649
### Gotcha
650650
651-
<!-- lore:019c8b78-965f-7f67-ae31-d24961133260 -->
652-
* **Codecov patch coverage requires --coverage flag on ALL test invocations**: The test script runs \`bun run test:unit && bun run test:isolated\`. Only \`test:unit\` has \`--coverage\`. Code paths exercised only in isolated tests won't count toward Codecov patch coverage. Fix: add \`--coverage\` to the isolated script, or write unit tests calling real (non-mocked) functions where possible.
653651
<!-- lore:ce43057f-2eff-461f-b49b-fb9ebaadff9d -->
654652
* **Sentry /users/me/ endpoint returns 403 for OAuth tokens — use /auth/ instead**: The Sentry \`/users/me/\` endpoint returns 403 for OAuth tokens. Use \`/auth/\` instead — it works with ALL token types and lives on the control silo. In the CLI, \`getControlSiloUrl()\` handles routing correctly. \`SentryUserSchema\` (with \`.passthrough()\`) handles the \`/auth/\` response since it only requires \`id\`.
655653
<!-- lore:8c0fabbb-590c-4a7b-812b-15accf978748 -->
@@ -664,8 +662,6 @@ mock.module("./some-module", () => ({
664662
* **Multiregion mock must include all control silo API routes**: When changing which Sentry API endpoint a function uses (e.g., switching getCurrentUser() from /users/me/ to /auth/), the mock route must be updated in BOTH test/mocks/routes.ts (single-region) AND test/mocks/multiregion.ts createControlSiloRoutes() (multi-region). Missing the multiregion mock causes 404s in multi-region test scenarios. The multiregion control silo mock serves auth, user info, and region discovery routes. Cursor Bugbot caught this gap when /api/0/auth/ was added to routes.ts but not multiregion.ts.
665663
<!-- lore:5efa0434-09ff-49b6-b0a1-d8a7e740855b -->
666664
* **resolveCursor must be called inside org-all closure, not before dispatch**: In list commands using dispatchOrgScopedList with cursor pagination (e.g., project/list.ts), resolveCursor() must be called inside the 'org-all' override closure, not before dispatchOrgScopedList. If called before, it throws a ContextError before dispatch can throw the correct ValidationError for --cursor being used in non-org-all modes.
667-
<!-- lore:76c673bf-0417-47cb-a73d-9c941fbd182c -->
668-
* **handleProjectSearch ContextError resource must be "Project" not config.entityName**: In src/lib/org-list.ts handleProjectSearch, the first argument to ContextError is the resource name rendered as "${resource} is required.". Always pass "Project" (not config.entityName like "team" or "repository") since the error is about a missing project slug, not a missing entity of the command's type. A code comment documents the rationale inline.
669665
670666
### Pattern
671667
@@ -676,7 +672,7 @@ mock.module("./some-module", () => ({
676672
<!-- lore:019c8aed-4458-79c5-b5eb-01a3f7e926e0 -->
677673
* **Sentry CLI setFlagContext redacts sensitive flags before telemetry**: The setFlagContext() function in src/lib/telemetry.ts must redact sensitive flag values (like --token) before setting Sentry tags. A SENSITIVE\_FLAGS set contains flag names that should have their values replaced with '\[REDACTED]' instead of the actual value. This prevents secrets from leaking into telemetry. The scrub happens at the source (in setFlagContext itself) rather than in beforeSend, so the sensitive value never reaches the Sentry SDK.
678674
<!-- lore:019c9bb9-a797-725d-a271-702795d11894 -->
679-
* **Sentry CLI api command: normalizeFields auto-corrects colon separators with stderr warning**: The \`sentry api\` command's \`--field\`/\`--raw-field\` flags require \`key=value\` format. \`normalizeFields()\` auto-corrects \`:\` to \`=\` with a stderr warning when no \`=\` exists, but must skip JSON-shaped strings (starting with \`{\` or \`\[\`) to avoid silent data corruption. A \`--data\`/\`-d\` flag (curl convention) handles inline JSON bodies directly, mutually exclusive with \`--input\` and field flags. Positional args as implicit body was rejected: context-dependent behavior violates least surprise, ambiguity with mistyped endpoints, even curl uses explicit \`-d\`. When bare JSON is detected in a field value, auto-detect and use as body with a stderr hint suggesting \`--data\`. General pattern: auto-correct unambiguous user mistakes at command level with stderr warnings, keeping parsing functions pure.
675+
* **Sentry CLI api command: normalizeFields auto-corrects colon separators with stderr warning**: The \`sentry api\` command's \`--field\`/\`--raw-field\` flags require \`key=value\` format. \`normalizeFields()\` auto-corrects \`:\` to \`=\` with stderr warning, but skips JSON-shaped strings (starting with \`{\` or \`\[\`) to avoid data corruption. \`--data\`/\`-d\` flag (curl convention) handles inline JSON bodies, mutually exclusive with \`--input\` and field flags. \`extractJsonBody()\` scans fields for bare JSON, extracts as body, hints about \`--data\`. \`buildFromFields()\` orchestrates: normalize → extract JSON → route fields. Positional args as implicit body was rejected: context-dependent behavior violates least surprise. General pattern: auto-correct unambiguous user mistakes at command level with stderr warnings, keeping parsing functions pure.
680676
681677
### Preference
682678

src/commands/api.ts

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -897,11 +897,19 @@ export function buildFromFields(
897897
// Route remaining fields to body (merge) or params based on HTTP method
898898
const options = prepareRequestOptions(method, field, rawField);
899899
if (options.body) {
900-
// Merge field-built key=value entries into the auto-detected JSON body
900+
if (Array.isArray(body)) {
901+
// Can't meaningfully merge key=value fields into a JSON array body.
902+
throw new ValidationError(
903+
"Cannot combine a JSON array body with field flags (-F/-f). " +
904+
"Use --data/-d to pass the array as the full body without extra fields.",
905+
"field"
906+
);
907+
}
908+
// Merge field-built key=value entries into the auto-detected JSON object body
901909
body =
902-
body && typeof body === "object" && !Array.isArray(body)
910+
body && typeof body === "object"
903911
? { ...body, ...options.body }
904-
: (options.body ?? body);
912+
: options.body;
905913
}
906914

907915
return { body, params: options.params };
@@ -1035,6 +1043,17 @@ export const apiCommand = buildCommand({
10351043
);
10361044
}
10371045

1046+
if (
1047+
flags.data !== undefined &&
1048+
(flags.field?.length || flags["raw-field"]?.length)
1049+
) {
1050+
throw new ValidationError(
1051+
"Cannot use --data with --field or --raw-field. " +
1052+
"Use --data/-d for a full JSON body, or -F/-f for individual fields.",
1053+
"data"
1054+
);
1055+
}
1056+
10381057
if (flags.data !== undefined) {
10391058
body = parseDataBody(flags.data);
10401059
} else if (flags.input !== undefined) {

test/commands/api.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1398,4 +1398,22 @@ describe("buildFromFields", () => {
13981398
expect(result.body).toEqual({ status: "resolved" });
13991399
expect(stderr.output).toBe("");
14001400
});
1401+
1402+
test("throws ValidationError when JSON array body is mixed with field flags", () => {
1403+
const stderr = createMockWriter();
1404+
expect(() =>
1405+
buildFromFields(
1406+
"PUT",
1407+
{ "raw-field": ["[1,2,3]"], field: ["extra=field"] },
1408+
stderr
1409+
)
1410+
).toThrow(ValidationError);
1411+
expect(() =>
1412+
buildFromFields(
1413+
"PUT",
1414+
{ "raw-field": ["[1,2,3]"], field: ["extra=field"] },
1415+
stderr
1416+
)
1417+
).toThrow(/Cannot combine a JSON array/);
1418+
});
14011419
});

test/e2e/api.test.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,4 +333,76 @@ describe("sentry api", () => {
333333
},
334334
{ timeout: 15_000 }
335335
);
336+
337+
test(
338+
"--data and --input are mutually exclusive",
339+
async () => {
340+
await ctx.setAuthToken(TEST_TOKEN);
341+
342+
const result = await ctx.run([
343+
"api",
344+
"organizations/",
345+
"--method",
346+
"PUT",
347+
"--data",
348+
'{"name":"test"}',
349+
"--input",
350+
"-",
351+
]);
352+
353+
expect(result.exitCode).toBe(1);
354+
expect(result.stderr + result.stdout).toMatch(
355+
/--data.*--input|--input.*--data/i
356+
);
357+
},
358+
{ timeout: 15_000 }
359+
);
360+
361+
test(
362+
"--data and --field are mutually exclusive",
363+
async () => {
364+
await ctx.setAuthToken(TEST_TOKEN);
365+
366+
const result = await ctx.run([
367+
"api",
368+
"organizations/",
369+
"--method",
370+
"PUT",
371+
"--data",
372+
'{"name":"test"}',
373+
"--field",
374+
"slug=my-org",
375+
]);
376+
377+
expect(result.exitCode).toBe(1);
378+
expect(result.stderr + result.stdout).toMatch(
379+
/--data.*--field|--field.*--data/i
380+
);
381+
},
382+
{ timeout: 15_000 }
383+
);
384+
385+
test(
386+
"--data and --raw-field are mutually exclusive",
387+
async () => {
388+
await ctx.setAuthToken(TEST_TOKEN);
389+
390+
const result = await ctx.run([
391+
"api",
392+
"organizations/",
393+
"--method",
394+
"PUT",
395+
"-d",
396+
'{"name":"test"}',
397+
"-f",
398+
"slug=my-org",
399+
]);
400+
401+
expect(result.exitCode).toBe(1);
402+
expect(result.stderr + result.stdout).toMatch(
403+
/--data.*--field|--field.*--data/i
404+
);
405+
},
406+
{ timeout: 15_000 }
407+
);
336408
});

0 commit comments

Comments
 (0)