Repository navigation
fix(porcelain): report a malformed LFS pointer OID instead of panicking - #664
Dev-next-gen wants to merge 1 commit into
Conversation
A deploy with large media enabled reads every file in the publish directory as a possible Git LFS pointer. readLFSData took the second element of the OID split on ":", so a pointer whose `oid` line carries no `<hash method>:` prefix made the whole process die with "index out of range [1] with length 1" instead of failing the deploy with an error.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review. 📝 SummarySummary by CodeRabbit
Walkthrough
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~8 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The malformed pointer now produces an error instead of terminating deployment. No actionable merge-blocking issue remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
What
readLFSDatapanics on a Git LFS pointer whoseoidline has no<hash method>:prefix, so asingle malformed pointer file in the publish directory kills the whole deploy process instead of
failing it with an error.
How I ran into it
I was reading the large-media path of
DoDeployand noticed thatreadLFSDataguards a missingoidkey but not a missing separator:An LFS OID is
<hash method>:<hash>. Without the separator,strings.SplitNreturns aone-element slice and
[1]panics.Reaching it takes no special setup: with large media enabled,
walkhands every regular file inthe publish directory to
readLFSData, which only checks the 41-byte version header beforeparsing the rest. A file containing
(the pointer from the existing
TestGetLFSSha, minus itssha256:) takes the deploy down:The change
strings.Cutinstead of indexing a split, and amalformed LFS OID: %serror when the separatoris absent — consistent with the
missing LFS OIDerror right above it. A file that advertises theLFS header but carries an unreadable pointer is now reported rather than silently deployed as if
its own bytes were the content.
Tests
Both new tests fail before the change (they panic) and pass after:
TestGetLFSShasub-case for an OID with no hash method;TestDoDeploy_MalformedLFSPointer, which goes through the real entry point (DoDeploy→walk→readLFSData) and also asserts the deploy never reaches the API.go test ./go/porcelain/...is green.Unrelated to this change,
go vet ./go/porcelain/already reports fourlostcancelfindings onmaster(includingdeploy.go:572);make testdoes not run that analyzer, so they are invisiblein CI. I left them alone to keep this PR to one fix.
Found by a defect-hunting pipeline I build and run (Dev-next-gen), using Claude Code with Anthropic's Claude Opus 5.