Skip to content

assertScenarioLocalBranchInputSafe silently skips its forbidden-key/oversize checks when changedFiles is not an array #8328

Description

@JSONbored

Context

src/scenarios/input-model.ts's assertScenarioLocalBranchInputSafe (around line 189) is a safety-boundary validator: its error messages explicitly say "source contents are never uploaded" and "Refusing oversized changedFiles payload; metadata-only paths are required." Its changedFiles handling is:

const changedFiles = payload.changedFiles;
if (Array.isArray(changedFiles)) {
  for (const entry of changedFiles) {
    if (!entry || typeof entry !== "object") continue;
    for (const nestedKey of Object.keys(entry as Record<string, unknown>)) {
      if (FORBIDDEN_SOURCE_UPLOAD_KEYS.test(nestedKey)) {
        throw new Error(`Refusing changedFiles.${nestedKey}; source contents are never uploaded.`);
      }
      const value = (entry as Record<string, unknown>)[nestedKey];
      if (typeof value === "string" && value.length > 4000) {
        throw new Error("Refusing oversized changedFiles payload; metadata-only paths are required.");
      }
    }
  }
}

The Array.isArray(changedFiles) guard means that if a caller passes changedFiles as any non-array truthy value (e.g. a plain object like { diff: "…5000 chars of source…" }, or a string), the entire nested forbidden-key/oversize check is skipped silently — the function returns normally instead of throwing, even though the payload contains exactly the kind of forbidden content (diff, oversized string) the function exists to reject. The outer per-key loop over payload's own top-level keys does not recurse into a non-array changedFiles value, so nothing else in the function catches this either.

test/unit/scenario-input-model.test.ts (line 250-266) only ever passes changedFiles as an array (or omits it entirely), so this bypass has no test coverage today.

Requirements

  • assertScenarioLocalBranchInputSafe must reject a non-array, non-undefined changedFiles value outright (it is documented as an array-of-entries shape; anything else — a plain object, string, number, etc. — is not a valid changedFiles and should not be silently ignored). Throw an Error with a message following the function's existing style (e.g. `Refusing non-array changedFiles; an array of file entries is required.`).
  • Do not change behavior for the existing, correctly-handled cases: a valid array of entries (with or without forbidden keys/oversized values), an omitted changedFiles, or changedFiles: undefined.

Deliverables

  • assertScenarioLocalBranchInputSafe throws when payload.changedFiles is present and not an array (and not undefined).
  • A regression test in test/unit/scenario-input-model.test.ts asserting assertScenarioLocalBranchInputSafe({ changedFiles: { diff: "x".repeat(5000) } }) throws.
  • A regression test asserting assertScenarioLocalBranchInputSafe({ changedFiles: "not-an-array" }) throws.
  • Confirm the existing passing cases (valid array changedFiles, omitted changedFiles) still do not throw — do not regress test/unit/scenario-input-model.test.ts's existing assertions at lines 253-254, 256, 266.

Test Coverage Requirements

This repo's Codecov patch gate requires 99%+ coverage of changed lines and branches. Cover both the new "reject non-array changedFiles" branch and confirm the existing array-handling branches remain covered and passing.

Expected Outcome

assertScenarioLocalBranchInputSafe can no longer be bypassed by passing changedFiles as a non-array value — every shape of changedFiles is now either validated against the forbidden-key/oversize rules (if a valid array) or rejected outright (if not).

Links & Resources

  • src/scenarios/input-model.tsassertScenarioLocalBranchInputSafe, around line 189-211
  • test/unit/scenario-input-model.test.ts — existing coverage, lines 250-266

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions