docs(proguard): note SHA-1 is required for v5 UUID, not security#1059
Merged
Conversation
Follow-up to #1058: the clarifying comment landed in a commit that was not part of the squashed merge. SHA-1 is mandated by RFC 4122 §4.3 for name-based UUIDs and must match legacy sentry-cli mapping IDs; it is never used for security. CodeQL alert was dismissed as a false positive.
Contributor
|
Contributor
Codecov Results 📊✅ Patch coverage is 100.00%. Project has 4420 uncovered lines. Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 81.85% 81.85% —%
==========================================
Files 341 341 —
Lines 24358 24358 —
Branches 15912 15912 —
==========================================
+ Hits 19937 19938 +1
- Misses 4421 4420 -1
- Partials 1665 1664 -1Generated by Codecov Action |
BYK
added a commit
that referenced
this pull request
Jun 4, 2026
## Summary Follow-up to #1058 that addresses the review threads still **open** after the earlier follow-ups (#1059, #1063). Each thread is mapped below. ### Release commands — eliminate the silent `args.join(" ")` defect (thread [3349270396]) The sentry bot flagged that `release restore 1.0.0 extra` silently joins extra positionals into `"1.0.0 extra"`. This `args.join(" ").trim()` pattern was shared by **all** release commands, so rather than a one-off guard this fixes the whole class: - All version-only release commands (`archive`, `restore`, `create`, `delete`, `deploys`, `finalize`, `view`, `set-commits`) now declare a single `kind: "tuple"` positional, so **Stricli enforces arity** — extra args are rejected instead of silently swallowed. - `release deploy` becomes a 3-positional tuple (`<version> <environment> [name]`); multi-word deploy names must be quoted (e.g. `"Deploy #42"`), matching standard CLI conventions and the existing docs example. - New `resolveReleaseTarget()` helper in `src/commands/release/parse.ts` centralizes the version-parse + org-resolution + `ContextError` boilerplate. - **New GritQL lint rule** `lint-rules/no-args-join-in-release.grit` bans `args.join()` in `src/commands/release/` so the anti-pattern cannot return (verified it fires on a reintroduced `args.join`). ### `sourcemap resolve` — single counting loop (thread [3356439786]) Replaced two `files.filter(...)` passes with one `for...of` loop tallying both `resolved` and `withDebugId`. ### `sourcemap inject` — remote detection + sort (threads [3356454439], [3356458605]) - Remote sourcemap detection now uses an anchored `^https?://` regex instead of two `startsWith` checks. - Path sort uses byte-wise comparison instead of locale-dependent `localeCompare` (matching the plain `.sort()` already used in `upload.ts`). ### `proguard` — precompute namespace bytes (threads [3356471815], [3356474050]) `uuidV5()` reparsed the hyphenated namespace UUID into bytes on every call. The namespace is fixed, so the bytes are now precomputed once at module load. **Behavior-preserving** — golden vectors unchanged (`void\n` → `5db7294d-87fc-5726-a5c0-4a90679657a5`, namespace `4f44f30f-...`). ### Already handled (resolving with notes) - [3349270382] (resolved-count): the count was fixed in #1063 via `hasSourcemap()`. The inject-hint sub-point is a non-issue — `sourcemap inject` adds debug IDs to the JS file itself regardless of where its sourcemap lives, so the hint is correct. - [3349258944] (CodeQL weak-crypto): already resolved/dismissed via #1059. ## Testing - Full unit suite: **7449 passed / 0 failed / 13 skipped**. - `bun run typecheck`, `bun run lint` (incl. new GritQL rule), `check:fragments`, `check:errors` all green. - Added `resolveReleaseTarget` unit tests; updated the `release deploy` and `sourcemap resolve` tests for the new signatures. [3349270396]: #1058 (comment) [3356439786]: #1058 (comment) [3356454439]: #1058 (comment) [3356458605]: #1058 (comment) [3356471815]: #1058 (comment) [3356474050]: #1058 (comment) [3349270382]: #1058 (comment) [3349258944]: #1058 (comment)
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #1058. The SHA-1 clarifying comment was in a second commit that didn't make the squashed merge. Adds it to
src/lib/proguard.tsso the codebase documents why SHA-1 is used (RFC 4122 §4.3 name-based UUIDv5, required to match legacy sentry-cli mapping IDs — never for security). The CodeQLjs/weak-cryptographic-algorithmalert was dismissed as a false positive.Docs/comment-only; no behavior change.