fix(review): close SSRF and lockfile-tamper detection gaps - #7826
Conversation
…7777) ipv6IsPrivateOrLocal in safe-url.ts recognized the IPv4-mapped IPv6 form (::ffff:a.b.c.d, normalized by new URL() to ::ffff:7f00:1) but not the older, ffff:-less IPv4-compatible form (::a.b.c.d, normalized the same bracket-free way to ::7f00:1). A URL like https://[::169.254.169.254] (cloud metadata) or https://[::127.0.0.1] (loopback) passed the guard as a public host. Generalize the existing hex-pair-to-IPv4 conversion to treat "ffff:" as optional, so both encodings are checked the same way. Applied identically to the byte-identical engine twin (packages/loopover-engine/src/review/safe-url.ts) to keep engine-parity:drift-check passing. Closes #7777
#7778) scanPackageLockPatch tracked which package-lock entry a line belonged to only by watching for that entry's own opening "node_modules/<pkg>": { line in the diff. git's default 3-line context doesn't guarantee that line survives when a changed resolved/integrity/version field sits deeper into the entry -- when it doesn't, currentEntryKey stayed null for the whole hunk and the change was silently dropped instead of flagged. Add a fallback "unattributed entry" bucket for a tracked-field change with no known active entry, gated by a new insideRejectedBlock flag so the existing deliberate-skip case (a malformed "node_modules/" key with nothing after the marker) still behaves exactly as before. Closes #7778
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | f03cff9 | Commit Preview URL Branch Preview URL |
Jul 21 2026, 02:03 PM |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7826 +/- ##
==========================================
- Coverage 88.65% 88.57% -0.09%
==========================================
Files 730 725 -5
Lines 76959 76426 -533
Branches 22791 22764 -27
==========================================
- Hits 68229 67695 -534
+ Misses 7684 7681 -3
- Partials 1046 1050 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-21 14:33:06 UTC
Review summary Nits — 5 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Summary
safe-url.ts:ipv6IsPrivateOrLocalonly recognized the IPv4-mapped IPv6 form (::ffff:a.b.c.d), not the older,ffff:-less IPv4-compatible form (::a.b.c.d).new URL()normalizes both to the same bracket-free hex shape, sohttps://[::169.254.169.254](cloud metadata) andhttps://[::127.0.0.1](loopback) both passed the guard as "safe." Generalized the existing hex-pair-to-IPv4 conversion to make theffff:marker optional, applied identically to the byte-identical engine twin.scanPackageLockPatchtracked "which package entry am I in" purely by watching for that entry's opening"node_modules/<pkg>": {line in the diff text. git's default 3-line context doesn't guarantee that line survives when a changedresolved/integrity/versionfield sits deeper into the entry — when it doesn't, the change was silently dropped instead of flagged. Added a fallback "unattributed entry" bucket for a tracked-field change with no known active entry, gated so the existing deliberate-skip case (a malformed"node_modules/"key) still behaves exactly as before.Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
src/review/content-lane/safe-url.ts, its byte-identical engine twin,src/review/lockfile-tamper.ts, and their unit tests — no workflows, MCP, workers, or UI code changed, soactionlint,test:workers,build:mcp,test:mcp-pack, and theui:*checks are out of scope. Also rannpm run engine-parity:drift-check(passes — the twosafe-url.tscopies stay byte-identical) and confirmed 100% line+branch coverage on all three changed source files via the lcov report.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.UI Evidence
N/A — backend-only change, no visible UI/frontend/docs surface touched.
Notes
maintainer-only(self-host only, no live hosted/multi-tenant exposure at the time of filing) with the fix direction already scoped in the issue text; this PR fixes both directly rather than opening them to contributors, per each issue's own note.Closes #7777
Closes #7778