fix: prevent secret tracing in dependency build stages#4147
Conversation
|
Warning Review limit reached
Next review available in: 6 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthroughThe Dockerfile updates shell execution flags and cargo dependency-cache preparation in the ChangesDocker build cache shell handling
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
🕓 Ready for review — 37 ahead in queue (commit 475f09f) |
infraclaw-dash
left a comment
There was a problem hiding this comment.
Requesting changes because the credential can still be emitted to build logs on a Git failure.
Lines 467 and 889, plus the existing occurrence at line 645, embed GITHUB_TOKEN as the URL username. Git includes URL usernames in stderr. I reproduced this failure output with a synthetic token:
fatal: could not read Password for https://SYNTHETIC_TOKEN_DO_NOT_USE@github.com
Please use a non-secret username and put the token in the password position, for example https://x-access-token:TOKEN@github.com/, or use a credential helper. Git redacts URL passwords in errors. Please update all three occurrences.
The EXIT trap and removal of xtrace otherwise look correct. The Docker build checks were skipped, so this Dockerfile change has not been exercised by CI.
git redacts URL passwords but not URL usernames in its error output, so embedding the token as the username in the insteadOf rewrite leaked it on any fetch/auth failure even with xtrace disabled. Use the x-access-token:TOKEN form at all three sites so the token sits in the password slot git knows to redact. Also bring the build-js stage up to the same standard as the other two secret-mounting stages: -s guard instead of -f (an empty secret file previously configured a broken rewrite) and EXIT-trap cleanup instead of success-only rm. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
What
Why
The affected stages expanded a mounted credential while command tracing was enabled. Raw build output could therefore include credential material even though BuildKit kept the mounted file out of image layers.
This covers PLATFORM-DS-CAND-376.
Checks
A Docker build parser check was attempted, but the local Docker daemon is not running.
Summary by CodeRabbit