Skip to content

fix(miner): bound rejection policy document reads#5325

Merged
JSONbored merged 2 commits into
mainfrom
codex/fix-unbounded-policy-doc-fetch-vulnerability
Jul 12, 2026
Merged

fix(miner): bound rejection policy document reads#5325
JSONbored merged 2 commits into
mainfrom
codex/fix-unbounded-policy-doc-fetch-vulnerability

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • The rejection-signal path previously read attacker-controlled policy files with response.text() and no size checks, allowing a malicious repo to cause excessive memory consumption or crash the miner attempt.
  • Both AI-USAGE.md and CONTRIBUTING.md were fetched concurrently even when the first file already decided the verdict, increasing the attack surface and risk before a worktree slot is allocated.

Description

  • Add a MAX_POLICY_DOC_BYTES = 128 * 1024 cap and implement readBoundedPolicyDoc(response) that checks Content-Length, streams the body with byte accounting, cancels oversized streams, and falls back to a bounded text() read for non-streamable responses.
  • Replace the unbounded response.text() call with readBoundedPolicyDoc(response) inside fetchPolicyDoc.
  • Short-circuit policy resolution by fetching AI-USAGE.md first and only fetching CONTRIBUTING.md when AI-USAGE.md is empty/insufficient, avoiding unnecessary downloads.
  • Add unit tests covering short-circuiting behavior, Content-Length-based rejection without reading the body, and streamed-response cancellation when the byte limit is exceeded.

Testing

  • Ran npx vitest run test/unit/miner-rejection-signal.test.ts, and the new unit tests passed (11/11).
  • Ran npm run typecheck, which succeeded.
  • Ran npm run build:miner, which succeeded (node --check passed for miner libs).
  • Full npm run test:ci, npm audit --audit-level=moderate, and a complete unsharded npm run test:coverage could not be completed in this environment due to unrelated registry/worker-typegen/environment drift and global coverage thresholds; the focused unit/regression tests added for this change passed locally.

Codex Task

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 12, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui e816ab2 Commit Preview URL

Branch Preview URL
Jul 12 2026, 02:00 PM

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@JSONbored JSONbored self-assigned this Jul 12, 2026
@JSONbored JSONbored marked this pull request as draft July 12, 2026 14:03
@gittensory-orb gittensory-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 12, 2026
@gittensory-orb

gittensory-orb Bot commented Jul 12, 2026

Copy link
Copy Markdown

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-12 21:47:40 UTC

2 files · 1 AI reviewer · 2 blockers · readiness 93/100 · CI green · clean

⏸️ Suggested Action - Manual Review

Review summary
This PR closes a real DoS gap: `fetchPolicyDoc` previously called `response.text()` with no size limit on attacker-controlled raw content, and now routes through `readBoundedPolicyDoc`, which checks `Content-Length`, streams with byte accounting, and cancels the reader past 128KB. It also short-circuits the AI-USAGE.md/CONTRIBUTING.md double-fetch when AI-USAGE.md alone is non-empty, reducing attack surface before a worktree slot is allocated. The new tests (short-circuiting, Content-Length rejection without reading the body, streamed cancellation) exercise the real code paths shown in the diff and are not fabricated. The description does not cite or link an eligible open issue, which this repo's contribution policy requires for every external PR.

Blockers

  • The PR description doesn't close or clearly link an eligible open GitHub issue, which this repo's contributor policy requires before merge-readiness can be assessed.
  • packages/gittensory-miner/lib/rejection-signal.js:96 skips `CONTRIBUTING.md` for every non-empty `AI-USAGE.md`, so a repo with `AI-USAGE.md` containing neutral text like `See CONTRIBUTING.md` and `CONTRIBUTING.md` containing a ban now returns false; change this to parse the AI-USAGE verdict first and only skip CONTRIBUTING when that verdict is actually decisive, or keep fetching CONTRIBUTING for non-empty but non-ban AI-USAGE content.
Nits — 5 non-blocking
  • packages/gittensory-miner/lib/rejection-signal.js:44-45 — the non-streaming fallback (`!response.body?.getReader`) still calls `await response.text()` unbounded before checking size, so for any response shape lacking a readable stream the original unbounded-memory risk this PR fixes is still present (low practical risk since Node's undici fetch always exposes `body.getReader`, but worth a comment noting the residual gap or an explicit length-cap read there too).
  • packages/gittensory-miner/lib/rejection-signal.js:54-70 — 5 levels of nesting in `readBoundedPolicyDoc`'s streaming loop; consider extracting the accumulate-and-check step into a small helper for readability.
  • codecov/patch is at 92.59% against the 99% target — worth adding a case for a malformed/non-numeric `content-length` header (the `Number.isFinite(parsedLength)` false branch) to close the gap.
  • Confirm `resolveAiPolicyVerdict`'s actual precedence: the new short-circuit skips fetching CONTRIBUTING.md whenever `aiUsage` is any non-empty, non-whitespace string, even if that AI-USAGE.md doesn't contain a ban phrase — if the engine ever treats CONTRIBUTING.md as an independent signal rather than a strict fallback, a repo with a permissive AI-USAGE.md and a banning CONTRIBUTING.md would now be missed.
  • Add a test for the `Number.isFinite(parsedLength)` false/NaN branch on a malformed Content-Length header to close the coverage gap noted above.

Concerns raised — review before merging

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.

2. Maintainer requires a linked issue — Link the relevant issue (for example `Closes #123`) before opening the PR.
Signal Result Evidence
Code review ❌ 2 blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 44 registered-repo PR(s), 36 merged, 435 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 44 PR(s), 435 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, TypeScript, Ruby, Go, JavaScript, MDX, Shell, Solidity
  • Official Gittensor activity: 44 PR(s), 435 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
[BETA] Chat with Gittensory

Ask Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @gittensory ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @gittensory chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @gittensory mention with a real question is routed to the closest matching read-only command automatically -- no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

🟩 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 Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@codecov

codecov Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.71%. Comparing base (ecb18c1) to head (84371bd).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5325   +/-   ##
=======================================
  Coverage   94.71%   94.71%           
=======================================
  Files         555      555           
  Lines       44526    44550   +24     
  Branches    14664    14664           
=======================================
+ Hits        42172    42196   +24     
  Misses       1619     1619           
  Partials      735      735           
Flag Coverage Δ
shard-1 43.87% <0.00%> (-0.16%) ⬇️
shard-2 35.29% <3.70%> (+0.06%) ⬆️
shard-3 32.01% <100.00%> (+<0.01%) ⬆️
shard-4 31.54% <0.00%> (-0.54%) ⬇️
shard-5 33.10% <0.00%> (+0.37%) ⬆️
shard-6 43.73% <3.70%> (+0.25%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/gittensory-miner/lib/rejection-signal.js 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored JSONbored force-pushed the codex/fix-unbounded-policy-doc-fetch-vulnerability branch from e816ab2 to fd1425b Compare July 12, 2026 21:28
@JSONbored JSONbored marked this pull request as ready for review July 12, 2026 21:28
@gittensory-orb gittensory-orb Bot added the manual-review Gittensor contributor context label Jul 12, 2026
Covers the streaming reader's normal-completion path (never exercised --
existing tests only hit the non-stream fallback and the overflow-cancel
branch), plus the non-numeric content-length and oversized-non-streamed-body
branches.
@JSONbored JSONbored merged commit 1894b75 into main Jul 12, 2026
18 checks passed
@JSONbored JSONbored deleted the codex/fix-unbounded-policy-doc-fetch-vulnerability branch July 12, 2026 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aardvark codex gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant