fix: resolve open CodeQL alerts (shell command construction, table-cell escaping, workflow permissions) - #1026
Merged
Merged
Conversation
…ll escaping, workflow permissions) - record.ts: replace execSync template strings with execFileSync argument arrays (js/shell-command-injection-from-environment, alerts 10-12) - review renderers: escape backslashes before pipes in escapeTableCell, with tests wired into test:scripts (js/incomplete-sanitization, 36-37) - pr-code-security.yml: explicit permissions on the secret-detection job (actions/missing-workflow-permissions, alert 1) Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR adds secret-detection permissions, updates the script test target, switches CLI subprocesses to argument arrays, and improves Markdown escaping and code-span formatting in two renderers with tests. ChangesMarkdown table escaping
CLI process execution
Workflow and test selection
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
size-limit report 📦
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
skills-contrib/review-triage-phase/scripts/render-review-actions.test.mjs (1)
32-39: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover
targetFilescode-span rendering.The assertions cover table cells but not
targetFiles, which passes throughformatCodePaths(). Add a target file containing a literal backslash and assert that the rendered code span preserves one slash.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills-contrib/review-triage-phase/scripts/render-review-actions.test.mjs` around lines 32 - 39, Add coverage in the “escapes backslashes before pipes in table cells” test for targetFiles rendered through formatCodePaths(). Include a target file containing a literal backslash in buildPayload() and assert the rendered code span preserves exactly one backslash.skills-contrib/review-fetch-phase/scripts/render-review-state.test.mjs (1)
39-48: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd coverage for escaped source paths outside table cells.
This test verifies a table row but not
sourcePathinside the inline code span. Add a literal-backslashsourcePathassertion so the rendering regression is caught.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills-contrib/review-fetch-phase/scripts/render-review-state.test.mjs` around lines 39 - 48, Add an assertion in the test “escapes backslashes before pipes in table cells” that checks the rendered inline code span also preserves and escapes literal backslashes in sourcePath, using the existing markdown output and expected escaped path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/1-framework/3-tooling/cli/scripts/record.ts`:
- Around line 115-118: Update probeCliOutput to tokenize the command arguments
with shell-quoting support instead of splitting args on whitespace, while
continuing to invoke execFileSync without a shell. Preserve quoted values such
as "two words" as a single argv element and keep the existing command-prefix
removal behavior.
In `@skills-contrib/review-fetch-phase/scripts/render-review-state.mjs`:
- Line 78: Separate Markdown table-cell escaping from inline-code formatting:
update render-review-state.mjs at lines 78-78 to use a code-span-specific
formatter for sourcePath, and update render-review-actions.mjs at lines 87-87 to
use it for formatCodePaths() and sourcePath without escaping literal
backslashes. Add regression coverage for backslash-containing sourcePath in
render-review-state.test.mjs lines 39-48 and backslash-containing targetFiles in
render-review-actions.test.mjs lines 32-39, preserving the existing table
escaping behavior.
---
Nitpick comments:
In `@skills-contrib/review-fetch-phase/scripts/render-review-state.test.mjs`:
- Around line 39-48: Add an assertion in the test “escapes backslashes before
pipes in table cells” that checks the rendered inline code span also preserves
and escapes literal backslashes in sourcePath, using the existing markdown
output and expected escaped path.
In `@skills-contrib/review-triage-phase/scripts/render-review-actions.test.mjs`:
- Around line 32-39: Add coverage in the “escapes backslashes before pipes in
table cells” test for targetFiles rendered through formatCodePaths(). Include a
target file containing a literal backslash in buildPayload() and assert the
rendered code span preserves exactly one backslash.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 7ce604f0-7f46-44d5-b2da-30665bbdadbd
📒 Files selected for processing (7)
.github/workflows/pr-code-security.ymlpackage.jsonpackages/1-framework/3-tooling/cli/scripts/record.tsskills-contrib/review-fetch-phase/scripts/render-review-state.mjsskills-contrib/review-fetch-phase/scripts/render-review-state.test.mjsskills-contrib/review-triage-phase/scripts/render-review-actions.mjsskills-contrib/review-triage-phase/scripts/render-review-actions.test.mjs
@prisma-next/extension-author-tools
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/extension-arktype-json
@prisma-next/middleware-cache
@prisma-next/mongo
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/extension-postgis
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/sqlite
@prisma-next/extension-supabase
@prisma-next/target-mongo
@prisma-next/adapter-mongo
@prisma-next/driver-mongo
@prisma-next/contract
@prisma-next/utils
@prisma-next/config
@prisma-next/errors
@prisma-next/framework-components
@prisma-next/operations
@prisma-next/ts-render
@prisma-next/contract-authoring
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/psl-printer
@prisma-next/cli
@prisma-next/cli-telemetry
@prisma-next/config-loader
@prisma-next/emitter
@prisma-next/language-server
@prisma-next/migration-tools
prisma-next
@prisma-next/vite-plugin-contract-emit
@prisma-next/mongo-codec
@prisma-next/mongo-contract
@prisma-next/mongo-value
@prisma-next/mongo-contract-psl
@prisma-next/mongo-contract-ts
@prisma-next/mongo-emitter
@prisma-next/mongo-schema-ir
@prisma-next/mongo-query-ast
@prisma-next/mongo-orm
@prisma-next/mongo-query-builder
@prisma-next/mongo-lowering
@prisma-next/mongo-wire
@prisma-next/sql-contract
@prisma-next/sql-errors
@prisma-next/sql-operations
@prisma-next/sql-schema-ir
@prisma-next/sql-contract-psl
@prisma-next/sql-contract-ts
@prisma-next/sql-contract-emitter
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-builder
@prisma-next/target-postgres
@prisma-next/target-sqlite
@prisma-next/adapter-postgres
@prisma-next/adapter-sqlite
@prisma-next/driver-postgres
@prisma-next/driver-sqlite
commit: |
Backslashes are literal inside Markdown code spans, so routing code-span content through escapeTableCell doubled them. Add formatCodeSpan (escapes pipes for GFM table cells, leaves backslashes intact) and use it for the Source line in both renderers and for formatCodePaths. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
Contributor
Author
|
Done in c11fa7d (both nitpicks from the review body): render-review-state.test.mjs now asserts a literal-backslash sourcePath renders unchanged in the Source code span, and render-review-actions.test.mjs covers formatCodePaths() with a backslash-containing target file. Backing fix: formatCodeSpan escapes pipes but leaves backslashes literal. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills-contrib/review-triage-phase/scripts/render-review-actions.mjs`:
- Around line 93-104: Update formatCodeSpan() to choose a Markdown code fence
longer than the longest consecutive backtick run in the normalized text,
ensuring backtick-containing values cannot terminate the span; preserve existing
whitespace and pipe normalization. Add a regression test covering
backtick-containing paths passed through formatCodePaths().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: d33a529e-940f-4a55-878d-0435898f8b22
📒 Files selected for processing (4)
skills-contrib/review-fetch-phase/scripts/render-review-state.mjsskills-contrib/review-fetch-phase/scripts/render-review-state.test.mjsskills-contrib/review-triage-phase/scripts/render-review-actions.mjsskills-contrib/review-triage-phase/scripts/render-review-actions.test.mjs
🚧 Files skipped from review as they are similar to previous changes (3)
- skills-contrib/review-fetch-phase/scripts/render-review-state.test.mjs
- skills-contrib/review-triage-phase/scripts/render-review-actions.test.mjs
- skills-contrib/review-fetch-phase/scripts/render-review-state.mjs
Wrap code spans with a backtick fence longer than the longest backtick run in the content, space-padded when the content starts or ends with a backtick, so backtick-containing paths render as intact code spans. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
wmadden
approved these changes
Jul 22, 2026
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
3 participants
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.
Resolves the six pre-existing open CodeQL alerts on main. Every alert was a genuine finding; none is dismissed as a false positive.
Shell command construction in
record.ts(alerts #10, #11, #12 —js/shell-command-injection-from-environment)The recording script built shell command strings by interpolating absolute paths (
node ${CLI_BIN} …,vhs ${tapePath}). Per CodeQL's recommended remediation, all three sites now useexecFileSyncwith argument arrays, so no shell parses the paths:probeCliOutputsplits the config-sourced command on whitespace — everycommand:entry inrecordings/config.tsis plain space-separated tokens, so the split is faithful.runCliInalready had an args array and now passes it through directly. This also fixes latent mangling of any future arg containing spaces (previouslyjoin(' ')then shell-split).runVhsrunsvhswith the tape path as a single argument.The error path is unchanged:
execFileSyncfailures carry the samestdout/stderrproperties the catch block reads.Backslash escaping in review-table renderers (alerts #36, #37 —
js/incomplete-sanitization)escapeTableCellinrender-review-state.mjsandrender-review-actions.mjsescaped|but not\, so input likefoo \| bar(or a trailing backslash) produced\\|— a literal backslash followed by a live cell separator, corrupting the Markdown table. Backslashes are now escaped before pipes.Tests came first: new
node:testfiles next to each script drive the exported render functions with backslash+pipe payloads and assert the exact escaped rows. Both failed against the old code and are wired into the roottest:scripts, so they run in CI.Explicit workflow permissions (alert #1 —
actions/missing-workflow-permissions)The
secret-detectionjob inpr-code-security.ymlcalled the reusableprisma/.githubsecret-detection workflow with nopermissionsblock, defaulting to the repo's full token. It now grants exactly what the reusable workflow's job declares (contents: read,pull-requests: write); thecode-scanningjob already had its own block, so every job now carries explicit permissions.Alerts #24/#25 (
op-factory-call.ts) were already fixed by #1018 and are not touched here.Verification
Full local run on this branch: build, typecheck, the lint suite (incl.
lint:workflows,lint:skills),test:packages,test:scripts(222 pass, incl. the two new files), andfixtures:check— all green.🤖 Generated with Claude Code
Summary by CodeRabbit