fix(native): honour telemetry-safe segments only in the sub-view position - #2993
Conversation
…tion The allowlist applied at every path position, so a username that collides with a token — /bank is a valid profile link — survived redaction. A safe token is now kept only two segments after a route root (/qr/<code>/success, /add-money/<country>/bank); anywhere else it is an identifier. (cherry picked from commit d9a4ca9)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code-analysis diffPainscore total: 7167.8 → 7167.98 (+0.18) 🆕 New findings (3)
✅ Resolved (3)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Clean: telemetry-safe tokens are now retained only as sub-views under a declared route root, while claimable username collisions are redacted; exact-head functional CI passed.
Checked clean
- Verified the detached worktree HEAD, merge base, trusted author, dev base ref, and exact base/head SHAs against the supplied values.
- Reviewed the positional redaction logic, its telemetry caller, prefixed paths, bare profile links, nested profile identifiers, declared route roots, static sub-views, query/fragment stripping, and authority handling.
- Compared the collision cases with the paired API username validator: bank, crypto, and manteca are valid claimable usernames, while frontend route roots used as literals are server-reserved or invalid username shapes.
- Exact-head unit, native-export, typecheck, eslint, format, design-system lint, analyze, backend-baseline, provenance, aggregate CI, and review checks passed; screenshots and preview deployment were still in progress.
- The detached worktree has no installed Jest binary, so a duplicate local focused run was unavailable; git diff whitespace validation passed.
- No prior findings were supplied, and no issue comments or review comments were fetched or read.
Security review by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only and answers only security, privacy and money, so treat its findings as advice.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 13bdcbcdbcc3 · Context: repo, paired-api, ci · Took 6m
🖼 Visual diff — 5 screens moved7 of 66 shots changed · 59 identical · baseline
job summary · before/after/diff images — artifact Fixture screenshots, no backend. Advisory — this check never blocks a merge. Posted from the default branch by ds-shots-comment.yml; the report it renders is untrusted data. |
Follow-up to #2990, which merged at
f4d2b02e6a few minutes before this commit was pushed to its branch — so the fix never reacheddev. Cherry-pick ofd9a4ca9e6, unchanged.What
redactNativePathkept the telemetry-safe sub-view tokens (success,bank,manteca,crypto,us) at every path position. The API allows usernames likebank, and/<username>is a profile link, sohttps://peanut.me/banksurvived redaction and sent the username innative_link_received.raw. Raised by Chip (MINOR) and CodeRabbit (MAJOR, CWE-200) on the since-closed #2991.A safe token is now honoured only two segments after a route root —
/qr/<code>/success,/add-money/<country>/bank— and is an identifier anywhere else. Collision tests:/bank,/profile/crypto,/manteca/successall redact; the two legitimate sub-view shapes keep their token.Verification
native-routes.test: green on this branch (useNativeAppLinks.testunchanged and green on fix: review follow-ups — ds-lint font-(--x) weight, SupportDrawer hardware back, landing Log in link, claim receipt cancelledAt #2990's head).src/contentgitlink isdev's (f5990317) — no submodule drift.