Skip to content

fix: avoid public manifest cache poisoning - #922

Merged
JSONbored merged 1 commit into
mainfrom
codex/fix-public-manifest-cache-poisoning
Jun 19, 2026
Merged

fix: avoid public manifest cache poisoning#922
JSONbored merged 1 commit into
mainfrom
codex/fix-public-manifest-cache-poisoning

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • Public-only contributor-facing manifest loads were using the shared snapshot path and unconditionally persisting their fetched or empty fallback manifests into the shared repo-focus-manifest signal cache, which could shadow existing maintainer api_record snapshots and weaken private gate enforcement.
  • The change prevents low-privileged contributor endpoints from poisoning the shared cache and preserves private/API-backed manifest integrity for webhook and gate-resolution paths.

Description

  • Change loadRepoFocusManifestWithCachePolicy to skip persisting fetched or negative-cache snapshots when cachePolicy.publicOnly is true by wrapping persistRepoFocusManifest in if (!cachePolicy.publicOnly) { ... } in src/signals/focus-manifest-loader.ts.
  • Add a regression test does not let public-only loads overwrite API-backed private manifests to test/unit/focus-manifest-loader.test.ts that seeds an API-backed manifest, performs a public-only load, and asserts the private loader still returns the api_record snapshot.
  • Preserve existing behavior for normal/private loads so they continue to persist repo-file and negative-cache snapshots for performance.

Testing

  • Ran the unit tests for the focus manifest loader with npm test -- --run test/unit/focus-manifest-loader.test.ts, and the suite passed with all tests succeeding (23 passed).
  • The added regression test verifies the public-only path no longer overwrites API-backed private manifests and was included in the test run above.

Codex Task

@ghost

ghost commented Jun 19, 2026

Copy link
Copy Markdown

Important

Gittensory found maintainer review notes

Public GitHub metadata was checked for review readiness. Gittensor-specific context appears only when confirmed.

Readiness score: 93/100

Signal Result Evidence Action
Linked issue ⚠️ Missing No linked issue or no-issue rationale found. Explain no-issue PR.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found. No action.
Review load ✅ 20/20 Readiness component derived from cached public PR metadata and labels. No action.
Validation evidence ✅ 25/25 PR body includes validation/test evidence. No action.
Open PR queue ✅ 10/10 4 open PR(s), 0 likely reviewable, 4 unlinked. No action.
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 83 PR(s), 238 issue(s). No action.
Gate result ✅ Passing No configured blocker found. No action.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Review load = cached public PR metadata such as size labels, changed paths, and preflight status.
  • Open PR queue = repo-wide review pressure; it is not a PR quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
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, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 83 PR(s), 238 issue(s).
  • PR-specific overlap: none found.
Maintainer notes
  • No linked issue detected: The planned PR does not reference a closing issue or explicit linked issue number.
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.
  • Re-run Gittensory review

💰 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.

@ghost ghost added the gittensory:reviewed label Jun 19, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

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

@ghost

ghost commented Jun 19, 2026

Copy link
Copy Markdown

gittensory · advisory review

Reviewed 2 changed file(s) — two independent AI reviewers.

Changed files: src/signals/focus-manifest-loader.ts, test/unit/focus-manifest-loader.test.ts

Suggested action:Safe to merge — both reviewers found no blocking issues.

Reviewer A · gpt-oss-120b — recommends ✅ merge
The PR adds a guard to avoid persisting manifests when loading with a public‑only cache policy, preventing public loads from overwriting private API‑backed manifests, and includes a test confirming this behavior.

Suggestions

  • Consider adding a unit test that verifies persisting still occurs when cachePolicy.publicOnly is false, ensuring the original negative‑cache behavior remains intact.
  • Document the new publicOnly guard in the function comment or README to clarify its security intent.

Worth double-checking

  • Public‑only loads will no longer benefit from negative caching, potentially increasing fetch frequency for missing manifests.

Reviewer B · nemotron-3-120b-a12b — recommends ✅ merge
The PR adds a guard to prevent public-only manifest loads from persisting to the API-backed cache, thereby avoiding cache poisoning that could overwrite private manifests. The change is minimal, well-tested, and aligns with the project's public/private boundary invariant by ensuring public loads never update private storage.

Worth double-checking

  • Verify that all public-facing manifest loaders (like loadPublicRepoFocusManifest) correctly set cachePolicy.publicOnly: true to trigger the new guard.

@codecov

codecov Bot commented Jun 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.62%. Comparing base (f023e56) to head (799bdd5).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #922   +/-   ##
=======================================
  Coverage   96.62%   96.62%           
=======================================
  Files         109      109           
  Lines       14885    14886    +1     
  Branches     5381     5382    +1     
=======================================
+ Hits        14382    14383    +1     
  Misses        104      104           
  Partials      399      399           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored
JSONbored merged commit d34d021 into main Jun 19, 2026
20 checks passed
@JSONbored
JSONbored deleted the codex/fix-public-manifest-cache-poisoning branch June 19, 2026 06:32
@github-actions github-actions Bot mentioned this pull request Jun 19, 2026
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant