fix: improve error quality and prevent token leak in telemetry#279
Conversation
Three fixes targeting the highest-impact issues from the 0.11.0 release: - fix(issue): include issue ID in 'not found' error and suggest short-ID format (CLI-N, 118 events / 41 users). Catches 404 from getIssue() in both the 'numeric' and 'explicit-org-numeric' resolveIssue() cases and re-throws a ContextError with the ID and a hint to try <project>-<id>. - fix(telemetry): redact --token value from telemetry tags (CLI-19, security). setFlagContext() now replaces values of sensitive flags (currently: token) with '[REDACTED]' before calling Sentry.setTag(). - fix(issue list): validate --cursor format early (CLI-7H/7P/7B). Cursor values that are plain integers are now rejected at parse time with a message explaining the expected format. The --limit issue is already addressed on main via auto-pagination (#274).
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. Bug Fixes 🐛
Internal Changes 🔧
🤖 This preview updates automatically when you update the PR. |
Codecov Results 📊✅ Patch coverage is 100.00%. Project has 3591 uncovered lines. Files with missing lines (69)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 74.62% 74.71% +0.09%
==========================================
Files 115 115 —
Lines 14167 14200 +33
Branches 0 0 —
==========================================
+ Hits 10572 10609 +37
- Misses 3595 3591 -4
- Partials 0 0 —Generated by Codecov Action |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| throw new ContextError(`Issue ${parsed.id}`, commandHint, [ | ||
| `No issue with numeric ID ${parsed.id} found — you may not have access, or it may have been deleted.`, | ||
| `If this is a short ID suffix, try: sentry issue ${command} <project>-${parsed.id}`, | ||
| ]); |
There was a problem hiding this comment.
ContextError produces misleading "is required" for 404
Medium Severity
ContextError formats its first argument via buildContextMessage as "{resource} is required.", so passing "Issue ${parsed.id}" produces the message "Issue 12345 is required." followed by "Specify it using: sentry issue show <org>/12345". This is misleading for a 404 not-found error — the user did provide the issue ID, it just wasn't found. The actual helpful explanation ends up buried under the "Or:" section. Since this PR's goal is to improve error quality for these 404s, the use of ContextError undermines the intent.
Additional Locations (1)
| const USAGE_HINT = "sentry issue list <org>/<project>"; | ||
|
|
||
| /** Matches strings that are all digits — used to detect invalid cursor values */ | ||
| const ALL_DIGITS_RE = /^\d+$/; |
There was a problem hiding this comment.
Redundant regex duplicates existing isAllDigits utility
Low Severity
The new ALL_DIGITS_RE constant (/^\d+$/) is an exact duplicate of ALL_DIGITS_PATTERN in src/lib/utils.ts, which is already exposed as the isAllDigits() utility function. That same utility is already imported and used in the sibling file src/commands/issue/utils.ts. The cursor validation at line 747 could simply call isAllDigits(value) instead of introducing a second copy of the regex.
Additional Locations (1)
…r validation - test(issue/utils): 4 tests for resolveIssue() numeric and explicit-org-numeric 404 cases: ContextError thrown with ID + hint, non-404 errors propagate unchanged - test(telemetry): 3 tests for setFlagContext() SENSITIVE_FLAGS: token value is replaced with '[REDACTED]', tag is still set (presence signal preserved), non-sensitive flags alongside token are unaffected - test(issue/list): 4 tests for cursor flag parse function: 'last' accepted, valid opaque cursors accepted, plain integers rejected with descriptive error including the bad value and an example format
## Summary - Fixes **CLI-7W**: Users passing a numeric Sentry project ID (e.g., `sentry event view 7275560680 <event-id>`) got a confusing "Project not found" error - Also resolves **CLI-80** as already fixed by PR #279 (cursor format validation) ## Changes **`findProjectsBySlug` (api-client.ts):** When the input is all-digits, accept the API result even when the returned slug differs from the input. The Sentry API's `project_id_or_slug` parameter already resolves numeric IDs — the CLI just wasn't accepting those results. **`resolveProjectBySlug` (resolve-target.ts):** - When a numeric ID resolves successfully, prints a stderr hint: `Tip: Resolved project ID 7275560680 to acme/frontend. Use the slug form for faster lookups.` - When resolution fails with an all-digit input, the error message says "Numeric project IDs are not supported" instead of the generic "Check that you have access" **View commands:** Pass `this.stderr` to `resolveProjectBySlug` so the hint can be displayed. ## Testing - 2 new tests for `findProjectsBySlug`: numeric ID resolution accepted, non-numeric slug mismatch still rejected - 1 updated test for `project view` func (new `stderr` parameter) - All 2010 tests pass


Summary
Fixes three of the highest-impact issues from the 0.11.0 release (sourced from Sentry production data).
fix(issue): actionable error when issue not found by numeric ID (CLI-N)
118 events / 41 users
getIssue()surfaces a bare 404 as a generic message with no indication of which ID failed. Users often confuse numeric Sentry group IDs (which require access) with short-ID suffixes (which are human-readable).Now
resolveIssue()catches the 404 in both thenumericandexplicit-org-numericcases and re-throws aContextErrorwith:<project>-<id>short-ID formatfix(telemetry): redact
--tokenvalue from telemetry tags (CLI-19)Security — token values were being sent to Sentry
setFlagContext()intelemetry.tswas forwarding the raw value of every flag as aflag.<name>tag. Forsentry auth login --token sntrys_..., this meant the actual API token was appearing in theflag.tokentag on every telemetry event.Adds a
SENSITIVE_FLAGSset (currently:token) and replaces those tag values with[REDACTED]before the tag is set.fix(issue list): validate
--cursorformat before hitting the API (CLI-7H/7P/7B)21 events / 12 users — 400 Bad Request from API
--cursor 100(a plain integer) was forwarded directly to the API, which rejects it. Cursor values are opaque strings like1735689600:0:0. Plain-integer inputs are now rejected at parse time with a message explaining the expected format.Note: the
--limitpart of this bug is already addressed on main via auto-pagination (#274).