Repository navigation
docs(adr): accept ADR-0043; record the CodeQL findings its zod bump surfaced - #247
Merged
Merged
Conversation
…urfaced ADR-0043: status accepted, ratifiedBy @mbeacom. All five action items were already complete, including the post-deploy check of both served schema versions. Consequences gains a note on CodeQL alerts 9 and 10 (js/bad-code-sanitization in packages/ci/dist/index.js and queue-action.js). Both point at zod 4.6's object fast path, Doc.compile(), which #241's bump brought into the bundles. The flagged value is JSON.stringify of a zod shape key. Every such key is a field name from adrkit's static schemas, never corpus or PR content, and it only ever sits in a string-literal position in code run on Node. So both alerts are false positives. They are to be dismissed rather than excluded, because codeql.yml scans the committed bundles on purpose. MANIFEST.md regenerated by `bun run emit:manifest`. Signed-off-by: Mark Beacom <m@beacom.dev>
Decisions governing this change
Active proposals touching this changeThese are not yet ratified and do not bind this change:
|
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The documentation is internally consistent, but the claimed security-alert dismissals require verification by someone with code-scanning access.
Review effort: Balanced
Findings: None
What changed in this PR
Accepts ADR-0043 and documents why two Zod-related CodeQL findings are false positives.
Changes:
- Marks ADR-0043 accepted with human ratification.
- Records CodeQL analysis, dismissal rationale, and completed deployment verification.
- Regenerates the ADR inventory.
The alert dismissal state could not be verified because the GitHub API denied access.
| File | Description |
|---|---|
MANIFEST.md |
Updates generated status counts and ADR-0043’s status. |
docs/adr/0043-…md |
Records acceptance, ratification, CodeQL consequences, and completed actions. |
5 of 6 tasks
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
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.
What and why
ADR-0043 is now
accepted(ratifiedBy: @mbeacom). All five action items were already done, including the post-deploy check that both served schema versions have the right SHA-256.One new Consequences bullet records how CodeQL alerts #9 and #10 (
js/bad-code-sanitization) were handled. #241'szod4.6.5 bump surfaced them.Doc.compile(), which builds a parser withnew Function. The flagged code is bundled intopackages/ci/dist/index.jsandpackages/ci/dist/queue-action.js.JSON.stringifyof a zod object-schema key. Every such key is a field name from adrkit's own fixed schemas; no ADR, PR or event content can become one.z.recordkeys don't go through this code, and adrkit never callsz.compile.</script>and the U+2028/U+2029 line separators. These values only land inside JS string literals run on Node, where</script>is harmless and those two characters have been legal since ES2019.codeql.ymlscans the committed bundles on purpose. The bullet says to re-check that the schema keys are still fixed if a laterzodbump raises the alerts again.The dismissals themselves happen through the GitHub API, not in this PR. The bullet describes them as done ("are dismissed"), so do them before this merges.
MANIFEST.mdregenerated withbun run emit:manifest. ADR-0043's decision text is unchanged; the edits are status, provenance and the Consequences bullet.Checklist
adr lintpasses (43 records),MANIFEST.mdis regenerated, andcheck:stale-refspasses.Notes for reviewers
z.config({ jitless: true })hardening. It turns off thenew Functioncall at runtime, but the code stays in the bundle, so the alerts would stay open. It would be a separate hardening decision, not a fix for these alerts.paths-ignoreforpackages/ci/dist/**: those bundles are what runs in consumers' CI, and the workflow says it scans them on purpose.