fix(scenarios): reject non-array changedFiles that bypasses the source-upload scan - #8467
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8467 +/- ##
==========================================
- Coverage 92.45% 89.62% -2.84%
==========================================
Files 791 98 -693
Lines 79320 22790 -56530
Branches 23954 3898 -20056
==========================================
- Hits 73335 20425 -52910
+ Misses 4858 2187 -2671
+ Partials 1127 178 -949
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-24 14:51:38 UTC
Review summary Nits — 3 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.
|
What
assertScenarioLocalBranchInputSafeis a safety-boundary validator — it exists to refusesource content (
"source contents are never uploaded") and oversized payloads(
"metadata-only paths are required") in scenario local-branch inputs. But itschangedFileshandling was gated on
Array.isArray(changedFiles): a present-but-non-array value (aplain object like
{ diff: "…5000 chars of source…" }, or a bare string) fell through theArray.isArrayguard entirely, so the nested forbidden-key/oversize scan never ran and thefunction returned normally — silently accepting exactly the kind of payload it exists to
reject. The outer top-level-key loop only inspects
payload's own keys; it does not recurseinto a non-array
changedFilesvalue, so nothing else caught it either.Change
Reject a present, non-array
changedFilesoutright, before theArray.isArrayblock:Behavior for every already-correct case is unchanged: a valid array of entries (with or
without forbidden keys / oversized values) still runs the full scan, and an omitted /
undefinedchangedFilesis still allowed.Validation
test/unit/scenario-input-model.test.ts: bothassertScenarioLocalBranchInputSafe({ changedFiles: { diff: "x".repeat(5000) } })andassertScenarioLocalBranchInputSafe({ changedFiles: "not-an-array" })now throw, while avalid array and an omitted
changedFilesstill do not.npx vitest run test/unit/scenario-input-model.test.ts→ all pass; 100% branch coverage onthe new guard (both operands, both sides, verified via lcov).
npm run test:changednet green (modulo the pre-existing Windows-onlysubprocess/symlink baseline failures that pass on Linux CI).
Closes #8328