fix(cli): escape line separators in generated literals; license headers and review docs - #347
Conversation
…ty review record Closes the repo-fixable OpenSSF Best Practices gold criteria: - Every TypeScript source file, both launchers, and the HCL wrapper template now open with a copyright statement and an SPDX license identifier (copyright_per_file, license_per_file). The header sits in the template as well as the generated wrappers so hclSyncCheck stays in agreement, and tests/license_headers_test.ts fails the suite if a new source file ships without the header. - CONTRIBUTING.md documents the code review requirements: how review is conducted (gate, AI reviewers, human maintainer), what must be checked, and what is required for a change to be acceptable (code_review_standards). - docs/security-review.md records the performed security reviews with scope, method, confirmed findings, and outcomes, tied to the assurance case's requirements and boundaries (security_review). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011SWEqodqknyrDiwLsmpGEq
|
🤖 Zuke AI review 🔎 security review —
|
|
🤖 Zuke AI review 🔎 generic review —
|
Makes the per-file header a written coding guideline so every new file carries it from the start: the exact two lines, placement after any shebang, the comment form for shell files, and the rule that generated files get the header from their template. Enforcement already exists in the license headers test; this documents the requirement where agents and contributors read the rules. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011SWEqodqknyrDiwLsmpGEq
|
🤖 Zuke AI review 🔎 security review —
|
|
🤖 Zuke AI review 🔎 generic review —
|
The CodeQL check on the header PR surfaced the repository's open alerts: 28 results across three rules, reproduced and re-verified to zero with a local run of the same default code-scanning suite. - js/bad-code-sanitization: the import scaffolder built generated string literals with bare JSON.stringify, which leaves U+2028/U+2029 raw — legal in JSON, line terminators to a JavaScript parser — so a crafted task command could split a generated literal. They are now escaped, with a regression test. - js/incomplete-sanitization: the gitleaks summary table escaped pipes and control characters but not backslashes, so a value's own backslash could pose as an escape and fabricate cell content. Backslashes are escaped first now, with a regression test. - js/incomplete-url-substring-sanitization: test fakes and assertions routed on substring URL checks; they now match explicit https origin prefixes (or exact list membership), so an unexpected host can never satisfy them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011SWEqodqknyrDiwLsmpGEq
|
🤖 Zuke AI review 🔎 security review —
|
|
🤖 Zuke AI review 🔎 generic review —
|
| Finding | Reason |
|---|---|
| Generated-string escaping now depends on a narrower test path but lacks coverage for other import sources | The change only extends str() to escape U+2028/U+2029, and the new unit test exercises the actual caller translateCommand, so the claim that other import paths remain untested does not identify a concrete regression introduced by this diff. |
The change is mostly documentation and test hardening, with one small logic fix in Markdown escaping but no clear functional regression visible in the diff.
totollygeek
changed the title
|
🤖 Zuke AI review 🔎 security review —
|
| Finding | Reason |
|---|---|
| New security-review prose may normalize unverified claims without code backing | The new docs file is non-executable prose, and the diff already shows the referenced security-review content was added alongside code and test changes elsewhere, so there is no concrete security flaw or unverified runtime path to exploit. |
The change is mostly documentation and test hardening, with one minor review concern around new docs copied from untrusted prose and a low-risk string-escaping fix that appears safe.
|
🤖 Zuke AI review 🔎 generic review —
|
2 participants
What & why
Two things landed here: the OpenSSF gold-criteria groundwork, and — surfaced by CodeQL running on that large diff — fixes for every open CodeQL security alert in the repository (28 results across three rules, reproduced locally with the same default code-scanning suite and re-verified to zero).
CodeQL alert fixes
packages/cli/src/import.ts: the import scaffolder built generated string literals with bare JSON stringification, which leaves the U+2028 and U+2029 characters raw — legal in JSON strings, line terminators to a JavaScript parser — so a crafted task command could split a generated literal across lines. Those characters are now escaped, with a regression test. This is the release-worthy fix that titles the PR.build/gitleaks_report.ts: the findings table escaped pipes and control characters but not backslashes, letting a value's own backslash pose as an escape and fabricate cell content. Backslashes are escaped first now, with a regression test.Gold-criteria groundwork
tests/fails the suite on any source file missing it; the header lives in the template so the HCL sync check stays in agreement; the API docs check confirms the generated documentation is unaffected.docs/security-review.mddocuments the performed reviews with scope, method, findings, and outcomes.Related issues
Part of the OpenSSF Scorecard / Best Practices badge work (#339, #342, #346).
Checklist
packages/cli/.deno task cipasses locally: lint, fmt, type-check, tests, spell, the HCL sync check, and the API docs check.any, noascasts, no non-null assertions.🤖 Generated with Claude Code
https://claude.ai/code/session_011SWEqodqknyrDiwLsmpGEq