Skip to content

fix(miner): bound self-review manifest reads#5358

Merged
JSONbored merged 1 commit into
mainfrom
codex/propose-fix-for-unbounded-manifest-fetch
Jul 12, 2026
Merged

fix(miner): bound self-review manifest reads#5358
JSONbored merged 1 commit into
mainfrom
codex/propose-fix-for-unbounded-manifest-fetch

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • Prevent unbounded attacker-controlled manifest downloads in the miner by enforcing the same manifest byte cap used by the engine parser.

Description

  • Import the shared MAX_FOCUS_MANIFEST_BYTES from @jsonbored/gittensory-engine and use it in the miner manifest fetcher.
  • Replace the previous unbounded response.text() read with readBoundedManifestResponseText which rejects over-limit Content-Length, bounds fallback text reads by encoded byte length, and streams+cancels the reader when the cap is exceeded.
  • Keep the existing candidate-path fallback behavior intact and route successful raw-content responses through the bounded reader.
  • Add regression tests in test/unit/miner-self-review-context.test.ts that cover the Content-Length rejection and the streaming cancellation paths.

Testing

  • Ran npm run build:miner which completed successfully.
  • Ran npx vitest run test/unit/miner-self-review-context.test.ts and the updated unit suite passed (18 tests passed).
  • Ran npm run test:ci locally; it exercised the pre-submit checks but failed at cf-typegen:check due to a workspace drift (worker-configuration.d.ts is stale; run npm run cf-typegen).
  • Ran npm audit --audit-level=moderate which failed to complete due to the npm registry audit endpoint returning 403 Forbidden in this environment.

Codex Task

@JSONbored JSONbored self-assigned this Jul 12, 2026
@JSONbored JSONbored marked this pull request as draft July 12, 2026 14:54
@superagent-security

Copy link
Copy Markdown
Contributor

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

@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 0f04cee Commit Preview URL

Branch Preview URL
Jul 12 2026, 02:56 PM

@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:49:07 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 gap: the previous fetchManifestContent used unbounded response.text(), so a malicious raw-content manifest response could exhaust miner memory. The new readBoundedManifestResponseText correctly short-circuits on an over-limit Content-Length, and for chunked bodies it streams via the reader, accumulates byteLength, cancels once the cap is exceeded, and releases the lock in a finally block — matching the engine's existing MAX_FOCUS_MANIFEST_BYTES cap. The two new tests exercise the Content-Length rejection and the streaming-cancellation path against the real fetchSelfReviewContext entry point, not a fabricated internal call, so they cover the real path.

Blockers

  • packages/gittensory-miner/lib/self-review-context.js:217: This PR does not close or clearly link an eligible open issue for the miner manifest-read change; add a maintainer-authorized issue link such as `Fixes #NNNN` in the PR description, or explain why this is exempt from the repo's issue-scope rule.
Nits — 5 non-blocking
  • packages/gittensory-miner/lib/self-review-context.js:212-243 nests to depth 5 inside the try/while/if; consider extracting the per-chunk cap check into a small helper for readability.
  • The non-streaming fallback branch (no response.body.getReader, e.g. Node's undici in some test environments) uses response.text() then measures byte length after the fact — this still materializes the full oversized body in memory before rejecting it, which is a minor exception to the 'never over cap in memory' claim in the comment at self-review-context.js:213-214.
  • No test exercises the non-streaming (`!response.body?.getReader`) fallback path at self-review-context.js:220-223, so that branch's oversized-rejection behavior isn't directly verified.
  • Consider also capping the non-streaming fallback by reading in chunks or using a Content-Length-only rejection when reader support isn't available, to keep the 'never over cap in memory' guarantee airtight in all environments.
  • Add a direct unit test for readBoundedManifestResponseText covering the `!response.body?.getReader` branch to lock in its behavior.

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.34%. Comparing base (8cb5e74) to head (0f04cee).
⚠️ Report is 122 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5358   +/-   ##
=======================================
  Coverage   94.34%   94.34%           
=======================================
  Files         473      473           
  Lines       39982    39982           
  Branches    14576    14576           
=======================================
  Hits        37722    37722           
  Misses       1585     1585           
  Partials      675      675           
Flag Coverage Δ
shard-1 46.28% <ø> (-0.13%) ⬇️
shard-2 34.35% <ø> (-0.14%) ⬇️
shard-3 30.85% <ø> (-0.15%) ⬇️
shard-4 33.28% <ø> (-0.02%) ⬇️
shard-5 33.65% <ø> (-0.04%) ⬇️
shard-6 45.07% <ø> (+0.19%) ⬆️

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored JSONbored marked this pull request as ready for review July 12, 2026 21:34
@gittensory-orb gittensory-orb Bot added the manual-review Gittensor contributor context label Jul 12, 2026
@JSONbored JSONbored merged commit 09b155e into main Jul 12, 2026
19 checks passed
@JSONbored JSONbored deleted the codex/propose-fix-for-unbounded-manifest-fetch branch July 12, 2026 21:49
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