docs(privacy): disclose full-year ZIP correlation fields and retained JSON bytes - #105
Merged
Merged
Conversation
lamemustafa
marked this pull request as ready for review
July 31, 2026 18:11
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0c5fe9403
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… the disclosure Review of #105 found the README fix left three statements of the same facts disagreeing with it. docs/PRIVACY_QA.md still required the full-year ledger to contain "only" the old field list, so every release candidate following that checklist would have rejected the record the code actually writes, or skipped a check known to be stale. Its allowlist now matches the README entry, with a note that the two describe one record shape owned by the code — a difference between them is a defect in whichever is stale, not a judgement call. The Chrome Web Store justifications for `offscreen` and `scripting` described only PDF or Excel bytes, while GSTR-2B all-formats also stages portal-data JSON into OPFS. Both now say so, since that copy is what a Store reviewer reads. The new inventory test could not see the generated checkpoint family: it matched only `pack:` keys and iterated the two key objects, while `pack.artifact-acquisition.v2.*` is built from a prefix. Deleting its README entry left the suite green. The prefix is now exported as a canonical source and asserted directly — verified by removing the README line and watching the new case fail, then restoring it. Verified at this head: 99 files / 1390 tests with vitest run alone, plus tsc, eslint, prettier, wxt build, verify-extension-package and git diff --check. Signed-off-by: Tapish Khandelwal <tapishkhandelwal13@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 083d642ae4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…s it Review flagged that docs/PRIVACY_QA.md still told release reviewers to check only PDF/XLS staging, so a release could follow the privacy checklist and never verify JSON retention or cleanup. Rather than fix that one line, grepped for every statement of the same rule and found three more that named PDF/XLS and omitted portal-data JSON: the transient-byte prohibition in docs/PUBLICATION_READINESS.md, and both the diagnostic-exclusion and artifact-byte-boundary rules in docs/LIVE_FILED_RETURNS_SPIKE.md. All four now say the same thing. Fixing only the flagged instance would have left the identical defect in three files and invited another round for each — the rule stated for one case and not its siblings is the failure this repository keeps paying for. Also corrected the scripting justification, which was inaccurate rather than merely incomplete. The previous wording folded JSON into the action-bound interception paths, but filed-returns-json-acquisition.ts injects a MAIN-world function that issues its own authenticated same-origin fetch to a fixed portal JSON endpoint. That is a materially different way of reaching portal data, and a Chrome reviewer reads this text as the account of what the permission does, so the two paths are now described separately. Verified at this head: 99 files / 1390 tests with vitest run alone, plus tsc, eslint, prettier and git diff --check. Signed-off-by: Tapish Khandelwal <tapishkhandelwal13@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0203788715
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two more findings, both caused by the previous fix in this PR. The ledger allowlist I rewrote was itself a restated field list, and it drifted the other way: it permitted only identifiers and class fields while FiledReturnsDownloadDiagnostic also requires schemaVersion, eventType, returnType, financialYear, period and artifactType. Release QA following it would have rejected a valid ledger, which is the same harm as the stale version it replaced, in the opposite direction. So this stops enumerating. The checklist now points at the canonical contract and states the property QA actually needs to confirm — every admitted value is a user-chosen scope field, an opaque identifier, or a class rather than a raw value. Two revisions of one line drifted in two directions in a single PR; a third list would have been the same bet. Also separated JSON from the click-and-capture description. Adding JSON to the artifact-byte boundary pulled it under a paragraph that says Pack clicks a marked control and receives a generated blob, which is untrue for JSON: the injected main-world function issues its own authenticated same-origin fetch. Security and release reviewers were being pointed at the wrong behaviour to verify. Verified at this head: 99 files / 1390 tests with vitest run alone, plus prettier and git diff --check. Signed-off-by: Tapish Khandelwal <tapishkhandelwal13@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4766da025
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…n of it Third and fourth drift of the same checklist item, and the second finding is the instructive one: my previous fix pointed at the canonical *type* and asserted that matching it proves every value is opaque or a class. It does not. FiledReturnsDownloadDiagnostic types actionId and errorCategory as unrestricted strings, so a raw value satisfies the interface. Only isValidFiledReturnsDownloadDiagnosticState enforces a canonical action ID and an allow-listed error category. Referencing a canonical source is not enough if the source does not carry the property being claimed. The ledger list was also still wrong, omitting the persisted targetId and currentTargetId, which is the third direction this one item has drifted. So the item no longer describes the record at all. It now directs QA to run the two validators that own the shape — isFullFiscalYearLedger, which admits exactly LEDGER_KEYS and TARGET_KEYS through hasOnlyKeys, and the diagnostic state validator — plus an independent check for raw sensitive values. Those functions are exercised by the suite on every run; a prose list here is checkable by nothing, which is why it drifted three times in one pull request. Verified at this head: 99 files / 1390 tests with vitest run alone, plus prettier and git diff --check. Signed-off-by: Tapish Khandelwal <tapishkhandelwal13@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #103.
Correct the public Extension Storage disclosure and bind the registered Chrome storage-key inventory to the README.
This remains source-build alpha. No runtime behaviour changed.
Root Cause / Decision Record
The README asserted an exhaustive inventory, but it omitted persisted full-year ZIP correlation and safe diagnostic fields, and did not say retained OPFS staging can contain acquired portal-data JSON bytes.
This is the smallest safe fix: documentation now reflects the current record contracts, and a regression test reads the canonical exported key objects from
src/background/storage-keys.tsand requires every value to appear in theExtension Storagesection ofREADME.md.Scope
PACK_LOCAL_STORAGE_KEYSandPACK_SESSION_STORAGE_KEYS.Pack Workflow Preflight
pnpm workflow:preflightwas run before editing/push, or the skip reason is documented.Sanchika Adoption Gate
@sanchika/*packages or copied Sanchika guidance.../sanchika,sanchika/packages/*/src, or parent source paths.Privacy And Data-Flow Impact
Sensitive Surface Review
Chrome Web Store Impact
Verification
pnpm install --frozen-lockfilepnpm audit --audit-level high(not run; docs/test-only lane)pnpm exec wxt preparepnpm exec prettier --check .pnpm exec eslint . --max-warnings 0pnpm exec tsc --noEmitpnpm exec vitest run— 99 files / 1,389 tests passed.pnpm exec wxt build(not run; docs/test-only lane)node scripts/verify-extension-package.mjs .output/chrome-mv3(not run; docs/test-only lane)pnpm exec wxt zip(not run; docs/test-only lane)node scripts/verify-extension-zip.mjs(not run; docs/test-only lane)node scripts/write-release-provenance.mjs(not a release lane)node scripts/verify-github-release-assets.mjs --tag <tag> --zip <zip> --checksum <sha256> --provenance <json>(no release assets)node scripts/publish-chrome-web-store.mjs --zip .output/<zip> --provenance .output/pack-release-provenance.v1.json --publisher-id <id> --dry-run true(not a Store lane)git diff --checkpnpm review:gate -- --strict-head-review --required-review-author chatgpt-codex-connector --wait-head-review-ms 180000 --allow-missing-head-review(deferred until the draft PR has a head review; no merge/readiness claim)Artifact Evidence
d0c5fe9.PR Review Follow-Up
d0c5fe9Screenshots
Not applicable: documentation and test only.
DCO