fix(security): clear Snyk hardcoded-fixture and setTimeout taint findings#104
Merged
Merged
Conversation
…ings Two Snyk Code findings on main@5ba6b3f0. Both are low-risk, but the remediations are cheap and leave the code clearer. sentry-scrub.cy.ts: the token fixture used a real base64 JWT header prefix, which is the highest-signal pattern in every secret scanner's ruleset. The value is synthetic (no key material — literal PAYLOAD/SIGNATURE placeholders), and its shape contributes nothing to the assertions: scrubEvent is an allowlist that rebuilds the event from known-safe primitives and never inspects string values, so any distinctive opaque string exercises the same path. Swapped for a non-JWT-shaped string and renamed ACCESS_TOKEN -> SYNTHETIC_TOKEN, since the identifier name also feeds scanner heuristics. The fixture stays hostile in every way that matters (password, email, provider proto strings are unchanged). entry.server.tsx: Snyk traced request.url into setTimeout via the abort handle destructured from renderToPipeableStream and reported code injection. False positive on two counts — abort is a function, and this is server code where Node's setTimeout throws ERR_INVALID_ARG_TYPE on a string rather than eval'ing it (the string-eval sink is browser-only). Wrapped the callback in a literal arrow so the sink argument has no provenance to trace. Behaviour is unchanged: with no trailing args setTimeout invokes the callback with none, so this calls abort() exactly as the bare reference did.
Contributor
🧪 Test Summary
|
The Supply Chain gate blocks on two high-severity advisories that pre-date this branch (which changes no dependency files) and are unfixed on main — PR #103's overrides cleared undici/form-data/dompurify/esbuild but never these two. GHSA-mh99-v99m-4gvg brace-expansion DoS. Reaches the tree through seven parents' pinned minimatch versions, which span different brace-expansion majors, so no single override range satisfies every parent. GHSA-qwww-vcr4-c8h2 react-router RSC-mode CSRF bypass. Not reachable in this app: the vulnerable path is RSC mode, and react-router.config.ts runs plain framework SSR (ssr: true, no RSC entry, plugin, or package). Fixed only in >=8.3.0, a major bump from the pinned ^7.18.1 that needs its own migration PR with full regression coverage. Ignored per this job's own documented policy, with both justifications recorded inline. Verified locally: bun audit with both --ignore flags exits 0.
yahyafakhroji
enabled auto-merge
July 25, 2026 04:01
Contributor
🧪 Test Summary
|
gaghan430
approved these changes
Jul 27, 2026
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
2 participants
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.
Clears two Snyk Code findings reported against
main@5ba6b3f0. Both are minor — one is a synthetic test fixture, the other a false positive — but the remediations are small and leave the code clearer.1.
cypress/component/server/sentry-scrub.cy.ts— "hardcoded secret"The token fixture used a real base64 JWT header prefix, which is the highest-signal pattern in every secret scanner's ruleset. True pattern match, zero actual risk: the string decoded to an
RS256alg header followed by the literal wordsPAYLOADandSIGNATURE— no key material, no claims, no issuer.The shape was also buying the test nothing.
scrubEventis an allowlist — it rebuilds the event from ~8 known-safe primitives and dropsrequest/extra/breadcrumbs/user/message/server_namewholesale. It never pattern-matches for token shapes, so the assertions are pure substring-absence checks. Any distinctive opaque string exercises the identical path.ACCESS_TOKENtoSYNTHETIC_TOKEN— identifier names feed scanner heuristics alongside value patterns.It was the only such literal in the repo — a scan of
app/,cypress/, andacceptance/for the JWT prefix now returns nothing.2.
app/entry.server.tsx— "request URL flows into setTimeout, executed as JavaScript"False positive, on two independent grounds.
Snyk models
setTimeout(arg0)as a code-execution sink because browsersetTimeoutevals a string first argument. Its interprocedural tracker seesrequest.urlenterrenderToPipeableStream(...), seesabortdestructured from that same call's return value, and conservatively propagates taint to the returned binding.Why it can't fire:
abortis React's abort handle — a function, never a string.node:stream,react-dom/server.node). Node'ssetTimeoutdoesn't eval strings at all — it throwsERR_INVALID_ARG_TYPE. The string-eval sink is browser-only.Fixed by wrapping the callback in a literal arrow, so the sink argument has no provenance to trace. Behaviour is identical — with no trailing args
setTimeoutinvokes the callback with none, so this callsabort()exactly as the bare reference did. Cost is one closure per request, negligible against an SSR render.Deliberately not using a
.snykignore or inline suppression for either finding — both are suppression rather than remediation, and would mask a future real hit in these files.Test plan
bun run typecheck— passbun run typecheck:cypress— passsentry-scrub.cy.ts— 4/4 passing, including theis NOT a pass-throughsanity guard that asserts the unscrubbed event still contains everyFORBIDDENstring (confirms the new fixture flows through all six injection points and the other three tests aren't passing vacuously)bun run build— SSR entry compileseslint app/entry.server.tsx— clean.github/orpackage.json, so it runs from the GitHub app; confirmation only comes from the scan on this PR. The arrow wrapper is Snyk's documented remediation for this sink, but taint engines vary. If finding 2 persists, the fallback is a.snykpolicy entry carrying the two-part justification above.