feat(miner-extension): reject oversized pasted ranked-candidates JSON before saving - #5525
Conversation
… 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
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5525 +/- ##
=======================================
Coverage 94.79% 94.79%
=======================================
Files 566 566
Lines 45065 45065
Branches 14675 14675
=======================================
Hits 42718 42718
Misses 1613 1613
Partials 734 734
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Caution 🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥 🛑 Gittensory review result - reject/close recommendedReview updated: 2026-07-13 01:22:34 UTC
🛑 Suggested Action - Reject/Close Review summary Blockers
Nits — 5 non-blocking
Why this is blocked
📋 Copy for AI agents — paste into your coding agent
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
[BETA] Chat with GittensoryAsk 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.
Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands Visual preview
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.
|
|
Gittensory is closing this pull request on the maintainer's behalf (AI reviewers agree on a likely critical defect: apps/gittensory-miner-extension/options.js:17 accepts any JSON whose trimmed UTF-16 length is <= 8 MiB, so a valid array containing multibyte characters can pass `parseRankedCandidatesJson()` and still exceed `chrome.storage.local` quota at `chrome.storage.local.set`, recreating the silent/partial save failure this PR is meant to close; change the guard to measure the serialized byte size with a portable fallback or lower the bound to account for worst-case UTF-8 expansion and add a real-path test for non-ASCII input.). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed. |
Summary
textarea with no size bound at all. The extension does not request the
unlimitedStoragepermission, sochrome.storage.localis capped at its default ~10 MiBQUOTA_BYTESquota shared across every key — anoversized paste could silently fail to save (Chrome rejects the
set()call) or leave storage in a partialstate, with no clear feedback to the contributor about why.
options.js'sparseRankedCandidatesJsonnow checks the raw pasted text's length against a newMAX_RANKED_CANDIDATES_JSON_CHARSbound (8 MiB of UTF-16 characters) before attemptingJSON.parse— soan 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.setat all.text.length(UTF-16 code units), not aTextEncoder-based byte count: thiscontent script ships unbundled (no build step), and
TextEncoderalso isn't available in this repo'snode:vm-based unit-test harness for these scripts, so a plain length check keeps the logic portable anddirectly testable. 8 MiB of characters stays comfortably under the 10 MiB quota even accounting for multi-byte
UTF-8 expansion once actually persisted.
Fixes #4863
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally — this change lives entirely underapps/gittensory-miner-extension/**and its dedicated test filetest/unit/miner-extension-content.test.ts, outside vitest's rootcoverage.includeglob (only rootsrc/**is Codecov-measured), socodecov/patchcannot see this diff.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateRan the full local gate:
npm run test:ci(798 test files, 0 failures) andnpm audit --audit-level=moderate(0 vulnerabilities), both clean on the final rebased commit.Test coverage added to
test/unit/miner-extension-content.test.ts(now 21 tests total, up from 18): rejecting a payload one character over the bound with a clear "too large" error, and specifically asserting the size check fires beforeJSON.parseruns (proven with a non-JSON oversized string, checking the thrown error is not a JSON syntax error); accepting a payload of exactly the boundary length (MAX_RANKED_CANDIDATES_JSON_CHARSchars, not one over); and a regression test driving the real form-submit save flow end-to-end with an oversized paste, asserting the error surfaces throughshowStatusandchrome.storage.local.setis never called (zero partial writes).Safety
chrome.storageonly).chrome.storage.localwrite path.parseRankedCandidatesJson/showStatuscode, not a mock of the validation itself.UI Evidencesection below.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:ciincludes the full
test/unit/miner-extension-content.test.tssuite (21/21 passing), including a regression testthat drives the real options-page save flow with an oversized paste and asserts the visible status message
matches
/too large/iwhilechrome.storage.local.setis never invoked.Notes
text.lengthcheck overTextEncoder-based byte counting to keep this portable acrossthe unbundled content-script runtime and the repo's
node:vmtest harness for these files, at the cost ofbeing an approximation rather than an exact byte count — the 8 MiB bound leaves comfortable headroom under the
10 MiB
chrome.storage.localquota to absorb that approximation.unlimitedStoragepermission 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.