Skip to content

feat(miner-extension): reject oversized pasted ranked-candidates JSON before saving - #5530

Merged
loopover-orb[bot] merged 2 commits into
JSONbored:mainfrom
joaovictor712:feat/miner-extension-json-paste-size-limit
Jul 13, 2026
Merged

feat(miner-extension): reject oversized pasted ranked-candidates JSON before saving#5530
loopover-orb[bot] merged 2 commits into
JSONbored:mainfrom
joaovictor712:feat/miner-extension-json-paste-size-limit

Conversation

@joaovictor712

Copy link
Copy Markdown
Contributor

Summary

  • The miner extension accepted an arbitrarily large pasted JSON blob into the options page's ranked-candidates
    textarea with no size bound at all. The extension does not request the unlimitedStorage permission, so
    chrome.storage.local is capped at its default ~10 MiB QUOTA_BYTES quota shared across every key — an
    oversized paste could silently fail to save (Chrome rejects the set() call) or leave storage in a partial
    state, with no clear feedback to the contributor about why.
  • options.js's parseRankedCandidatesJson now measures the pasted text's real UTF-8 byte size via
    new TextEncoder().encode(trimmed).length and rejects anything over a new MAX_RANKED_CANDIDATES_JSON_BYTES
    bound (8 MiB) before attempting JSON.parse — so an oversized-but-invalid paste fails with a clear
    "too large" error rather than a confusing JSON syntax error, and a valid-but-oversized paste never reaches
    chrome.storage.local.set at all.
  • TextEncoder is a standard Web API available in both the real (unbundled) extension runtime and, once
    injected into the sandbox context, this repo's node:vm-based unit-test harness for these scripts — it is
    not present in a bare vm.createContext({}) by default, which the test helpers now account for.

Fixes #4863

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally — this change lives entirely under apps/gittensory-miner-extension/** and its dedicated test file test/unit/miner-extension-content.test.ts, outside vitest's root coverage.include glob (only root src/** is Codecov-measured), so codecov/patch cannot see this diff.
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

Ran the full local gate: npm run test:ci (798 test files, 0 failures) and npm audit --audit-level=moderate (0 vulnerabilities), both clean on the final commit.

Test coverage added to test/unit/miner-extension-content.test.ts (now 22 tests total, up from 18): rejecting a payload one byte over the bound with a clear "too large" error, and specifically asserting the size check fires before JSON.parse runs (proven with a non-JSON oversized string, checking the thrown error is not a JSON syntax error); accepting a payload of exactly the boundary byte length; a regression test driving the real form-submit save flow end-to-end with an oversized paste, asserting the error surfaces through showStatus and chrome.storage.local.set is never called (zero partial writes); and a dedicated regression test using "é"-character padding (1 UTF-16 code unit but 2 UTF-8 bytes each), sized so it would have wrongly passed a naive character-length check while its real byte size exceeds the bound — proving multibyte content can't sneak past the guard.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. — N/A, no auth/session/CORS surface touched (local chrome.storage only).
  • API/OpenAPI/MCP behavior is updated and tested where needed. — no public API/OpenAPI/MCP surface touched; this only guards a local chrome.storage.local write path.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. — the error path is driven through the real parseRankedCandidatesJson/showStatus code, not a mock of the validation itself.
  • Visible UI changes include a UI Evidence section below.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. — updated apps/gittensory-miner-extension/README.md's "Local ranked cache" section.

UI Evidence

This is a browser extension options page — there is no hosted/public deployment to screenshot from this
environment (no browser screenshot tooling available here). Verified functionally instead: npm run test:ci
includes the full test/unit/miner-extension-content.test.ts suite (22/22 passing), including a regression test
that drives the real options-page save flow with an oversized paste and asserts the visible status message
matches /too large/i while chrome.storage.local.set is never invoked.

Notes

  • A first attempt at this issue (PR feat(miner-extension): reject oversized pasted ranked-candidates JSON before saving #5525) was correctly auto-closed by the Gittensory gate: it measured the
    pasted text's UTF-16 .length (character count) instead of actual byte size, so a payload full of multibyte
    characters could pass the check yet still exceed the real chrome.storage.local quota once serialized,
    recreating the exact silent-failure bug the guard exists to prevent. This PR fixes that by measuring real
    UTF-8 bytes via TextEncoder instead, with the "é"-padding regression test above proving the fix — that
    same input would have passed the old, incorrect check.
  • Did not request the unlimitedStorage permission as an alternative fix; the issue's proposal was a bounded,
    validated paste path with a clear error, not expanding the extension's storage footprint.

… before saving

The extension doesn't request the unlimitedStorage permission, so
chrome.storage.local is capped at its default ~10 MiB quota with no
guard against an unbounded paste silently failing to save. Reject a
paste over a conservative size bound with a clear error, before ever
attempting to parse or save it.

Fixes #4863
…N size guard

A UTF-16 character-length check undercounts any multibyte content, so
a payload full of non-ASCII characters could pass the size guard added
in the previous commit yet still exceed chrome.storage.local's real
quota once serialized, recreating the exact silent-failure bug that
guard exists to prevent. Measure the actual UTF-8 byte size via
TextEncoder instead.
@joaovictor712
joaovictor712 requested a review from JSONbored as a code owner July 13, 2026 01:47
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.79%. Comparing base (05d1fb0) to head (8876431).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5530   +/-   ##
=======================================
  Coverage   94.79%   94.79%           
=======================================
  Files         566      566           
  Lines       45065    45065           
  Branches    14675    14675           
=======================================
  Hits        42718    42718           
  Misses       1613     1613           
  Partials      734      734           
Flag Coverage Δ
shard-1 43.65% <ø> (-0.56%) ⬇️
shard-2 35.53% <ø> (+0.06%) ⬆️
shard-3 32.11% <ø> (+0.16%) ⬆️
shard-4 31.35% <ø> (-0.58%) ⬇️
shard-5 33.32% <ø> (+0.44%) ⬆️
shard-6 43.72% <ø> (+0.20%) ⬆️

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.

@loopover-orb loopover-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 13, 2026
@loopover-orb

loopover-orb Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-13 01:56:25 UTC

3 files · 2 AI reviewers · no blockers · readiness 100/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR adds a UTF-8 byte-size guard (8 MiB) before JSON.parse in parseRankedCandidatesJson, correctly measuring real serialized byte size via TextEncoder rather than UTF-16 char length, which properly addresses the chrome.storage.local quota risk described in #4863. The fix is at the right layer (before parse and before storage.set), the test harness change to inject TextEncoder into the vm sandbox is necessary and correctly scoped, and the new tests exercise the real save flow including the multibyte-undercounting edge case. This is a small, well-targeted, narrowly-scoped defensive fix with no signs of scope creep.

Nits — 5 non-blocking
  • nits: the 8 MiB limit is somewhat arbitrary relative to the ~10 MiB shared quota — a brief note on why 8 (vs e.g. 9) was chosen would help future maintainers, though the existing comment in options.js:6-13 already covers most of the reasoning.
  • The 'at the bound' test in miner-extension-content.test.ts constructs `atLimit` using `.length` as a proxy for byte length, which only holds because the padding is single-byte ASCII 'x' — worth a one-line comment noting that assumption so a future edit to the padding character doesn't silently break the invariant being tested.
  • Consider referencing the exact chrome.storage.local QUOTA_BYTES constant (10485760) in the code comment instead of the rounded '10 MiB' for precision, apps/gittensory-miner-extension/options.js:6.
  • The regression test 'REGRESSION (gate-caught)' is good but could also assert the exact byteLength value in the thrown error message matches TextEncoder's actual count, for tighter coverage of the message-formatting path.
  • nit: apps/gittensory-miner-extension/options.js:6 has a long implementation-history comment in production code that duplicates the README and makes the simple bound harder to scan; keep the quota rationale but move the longer TextEncoder/test-harness explanation to the test or docs.
Signal Result Evidence
Code review ✅ No blockers 2 reviewers, synthesized
Linked issue ✅ Linked #4863
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 (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 1874 registered-repo PR(s), 1235 merged, 49 issue(s).
Contributor context ✅ Confirmed Gittensor contributor galuis116; Gittensor profile; 1874 PR(s), 49 issue(s).
Gate result ✅ Passing No configured blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence. LLM value judgment: moderate — The change closes a real silent-data-loss gap (unbounded paste vs capped chrome.storage.local quota) called out in the linked issue #4863, with correct UTF-8-based measurement and solid test coverage of the save-flow wiring.
Linked issue satisfaction

Addressed
The PR adds a byte-size check (8 MiB bound via TextEncoder) that rejects oversized pasted JSON with a clear error message before parsing or storage.set, directly matching the issue's ask for a bounded paste path with a clear overflow error, and includes tests verifying the size check and error surfacing through the save flow.

Review context
  • Author: galuis116
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: JavaScript, Python, Dart, TypeScript, HTML, MDX, Rust, C++
  • Official Gittensor activity: 1874 PR(s), 49 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
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 &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @gittensory chat &lt;question&gt; 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

Visual preview
Route Viewport Before (production) After (this PR's preview) Diff
/ desktop before / after /
/ mobile before / (mobile) after / (mobile)

Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy.

🟩 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

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gittensory approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit c81f314 into JSONbored:main Jul 13, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add storage-size validation for pasted JSON

1 participant