fix(security): resolve all 11 code scanning alerts - #3
Conversation
- workflows: stop interpolating workflow_run.head_branch into shell run steps; use head_sha via env and gate on same-repo runs (actions/code-injection, actions/untrusted-checkout) - battle tests: use execFileSync with an argument array instead of a shell command string (js/shell-command-injection-from-environment) - edit.ts: escape all ffmpeg filtergraph metacharacters in applyLUT (js/incomplete-sanitization) - screenshots.ts: reword screenshot-filename pattern to avoid nested quantifiers (js/polynomial-redos)
📝 WalkthroughWalkthroughThe workflows now check out exact same-repository workflow-run commits. Battle tests invoke FFmpeg without shell command construction. LUT paths escape additional filter characters. Screenshot placeholder matching is regrouped without behavior changes. ChangesWorkflow and media command updates
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@battle.test.ts`:
- Around line 51-52: Replace the string-based ffmpegExec wrappers in
battle.test.ts:51-52 and deno-tests/battle.test.ts:42-43 with readonly string[]
parameters, forwarding ['-y', ...args] directly to execFileSync without trimming
or splitting; update every ffmpegExec caller in both files to pass argument
arrays.
In `@lib/helpers/edit.ts`:
- Around line 611-612: Update the LUT path escaping in the helper that builds
the `lut3d` filter so apostrophes use FFmpeg-compatible quote-close,
escaped-apostrophe, and quote-reopen syntax rather than `\'` inside the outer
quotes. Preserve the existing escaping for the other filter-option characters
and continue wrapping the resulting path in `lut3d='...'`.
In `@lib/helpers/screenshots.ts`:
- Line 59: Align the filename-format handling between buildTimestampFilename and
executeTimestamps: ensure both accept and expand the same timestamp specifiers,
including formats such as %04.2d and %04i, or restrict executeTimestamps to
buildTimestampFilename’s supported syntax. Reuse a shared filename builder or
validator so identical patterns always produce identical paths.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5374b757-6071-4d38-a420-e3f3b774b9ad
📒 Files selected for processing (7)
.github/workflows/docs.yml.github/workflows/jsr.yml.github/workflows/publish.ymlbattle.test.tsdeno-tests/battle.test.tslib/helpers/edit.tslib/helpers/screenshots.ts
| function ffmpegExec(args) { | ||
| execSync(`ffmpeg -y ${args}`, { stdio: 'pipe' }); | ||
| execFileSync('ffmpeg', ['-y', ...args.trim().split(/\s+/)], { stdio: 'pipe' }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 4 'ffmpegExec|trim\(\)\.split|execFileSync' \
battle.test.ts deno-tests/battle.test.ts
rg -n -C 4 'p\s*\(|mkdtemp|makeTempDir|tmpdir|temp' \
battle.test.ts deno-tests/battle.test.tsRepository: GlobalTechInfo/mediaforge
Length of output: 50382
Replace the string-based FFmpeg wrappers with array APIs.
execFileSync does not run FFmpeg through a shell, so trimming and whitespace-splitting the command string will incorrectly split arguments. Paths containing spaces and filter expressions split too. Change ffmpegExec in both battle.test.ts and deno-tests/battle.test.ts to accept readonly string[] and forward ['-y', ...args] directly to execFileSync, then update each caller to pass arrays.
📍 Affects 2 files
battle.test.ts#L51-L52(this comment)deno-tests/battle.test.ts#L42-L43
🤖 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 `@battle.test.ts` around lines 51 - 52, Replace the string-based ffmpegExec
wrappers in battle.test.ts:51-52 and deno-tests/battle.test.ts:42-43 with
readonly string[] parameters, forwarding ['-y', ...args] directly to
execFileSync without trimming or splitting; update every ffmpegExec caller in
both files to pass argument arrays.
| const escapedLut = sanitised.replace(/(['\\,;\[\]%:])/g, '\\$1'); | ||
| const filter = `lut3d='${escapedLut}':interp=${interp}`; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
fd -a 'edit\.ts$' . || true
echo "== excerpt =="
if [ -f lib/helpers/edit.ts ]; then
wc -l lib/helpers/edit.ts
sed -n '560,640p' lib/helpers/edit.ts | cat -n
fi
echo "== search lut3d/lut generation and caller path =="
rg -n "lut3d|lut3d=|lut\(|build.*filter|filter3d|grade|\.cube" lib/helpers/edit.ts lib 2>/dev/null | head -200
echo "== call graph / relevant usages =="
rg -n "add.*lut|lut3d\(|lut3d=|sanitised|escapedLut" . --glob '!node_modules' --glob '!dist' --glob '!build' --glob '!coverage' | head -300
echo "== git diff stat/name-only if available =="
git diff --stat || true
git diff --name-only || trueRepository: GlobalTechInfo/mediaforge
Length of output: 4570
🌐 Web query:
FFmpeg filtergraph quoted options escaping single quote apostrophe quote close escape reopen documentation
💡 Result:
In FFmpeg, filtergraph options require multi-level escaping because special characters (such as ':', ',', '[', ']', ';', and the quote character itself) are parsed at different stages [1][2][3]. 1. The FFmpeg Parsing Rules FFmpeg uses the backslash () as the primary escape character [2]. Within a string, you can escape a special character by prefixing it with a backslash (e.g., : or ') [2]. Additionally, you can enclose strings in single quotes ('...') to include special characters literally [2]. However, since the quote character (') cannot be escaped inside single-quoted strings, you must close the quote, escape the apostrophe, and then reopen the quote: 'String with an ''' apostrophe' [2]. 2. Multi-Level Escaping Requirements When defining a complex filtergraph, you must account for multiple layers of interpretation [1][3]: - Filter Option Level (First Level): This stage parses the internal options of a single filter (e.g., the text value in a drawtext filter) [1][3]. Here, you escape special characters like the colon (:) or the quote character (') [1][3]. - Filtergraph Description Level (Second Level): If the filter description itself is embedded in a larger chain, you must escape characters that separate filters or streams, such as commas (,), brackets ([]), and semicolons (;) [1][3]. - Shell/Command Line Level (Third Level): Finally, when providing the filtergraph to a shell (like Bash or Zsh), you must apply escaping rules required by that specific shell [1][3]. This often requires backslash-doubling or extra quoting, leading to long chains of backslashes (e.g., \\' for a single apostrophe) [1][4]. 3. Practical Example: Escaping an Apostrophe If you need to include a string containing a single quote (e.g., text='s movie) inside a filter: - At the filter option level: text='s movie becomes text='s movie [1]. - When embedded in a filtergraph, you must also escape the comma: text='s movie, is... [1]. - When passing this to a shell, you must escape the backslashes again, potentially resulting in: -vf "drawtext=text=\\'s movie" (depending on the shell) [1][3]. To avoid "escaping hell," it is highly recommended to read filtergraph descriptions from a text file using the format: -vf "filter_complex_script=filename.txt" [4][5]. You can also use the internal FFmpeg tool ffescape to help generate correctly escaped strings [2][6].
Citations:
- 1: https://ffmpeg.org/ffmpeg-filters.html
- 2: https://ffmpeg.org/ffmpeg-utils.html
- 3: https://manpages.ubuntu.com/manpages/noble/man1/ffmpeg-filters.1.html
- 4: https://ffmpeg.org/pipermail/ffmpeg-user/2014-January/019505.html
- 5: https://stackoverflow.com/questions/79583803/how-to-quote-a-file-name-with-a-single-quote-in-ffmpeg-movie-filter-notation
- 6: https://manpages.debian.org/unstable/ffmpeg/ffmpeg-utils.1.en.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== FFmpegBuilder outline =="
ast-grep outline lib/FFmpeg.ts || true
echo "== FFmpegBuilder relevant sections =="
wc -l lib/FFmpeg.ts
sed -n '1,180p' lib/FFmpeg.ts | cat -n
echo "== filters/complex outline and relevant sections =="
ast-grep outline lib/filters/complex.ts || true
sed -n '1,160p' lib/filters/complex.ts | cat -n
echo "== behavioral probe: current escaping as FFmpeg filter option token =="
python3 - <<'PY'
def current_filter(lut, interp="trilinear"):
is_windows = False
sanitised = is_windows and lut or lut.replace("\\", "/")
escaped_lut = sanitised.replace(/(['\\,;\[\]%:])/g, '\\$1') if False else sanitised.replace(r"['\\,;\[\]%:]", lambda m: "\\" + m.group(0))
PY
python3 - <<'PY'
import re
def current_filter(lut, interp="trilinear"):
is_windows = False
sanitised = is_windows and lut or lut.replace("\\", "/")
escaped_lut = re.sub(r"(['\\,;\[\]%:])", r"\\\1", sanitised)
return f"lut3d='{escaped_lut}':interp={interp}"
def proposed_filter(lut, interp="trilinear"):
is_windows = False
sanitised = is_windows and lut or lut.replace("\\", "/")
escaped_lut = re.sub(r"\\", r"\\\\", sanitised)
escaped_lut = re.sub(r"'", r"'\''", escaped_lut)
escaped_lut = re.sub(r"([,;\[\]%:])", r"\\\1", escaped_lut)
return f"lut3d='{escaped_lut}':interp={interp}"
cases = ["film.cube", "/tmp/grade's.cube", "/tmp/a;b.cube", "/tmp/a\\b.cube", '/tmp/trap\'\'\'".cube']
for case in cases:
print("lut =", repr(case))
print("current=", current_filter(case))
print("proposed=", proposed_filter(case))
PYRepository: GlobalTechInfo/mediaforge
Length of output: 15718
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== videoFilter implementation =="
rg -n "videoFilter|addFilter|filterArgs|arguments" lib/FFmpeg.ts lib/filters/complex.ts lib/types/filters.ts 2>/dev/null
echo "== buildArgs run FFmpeg section =="
sed -n '240,360p' lib/FFmpeg.ts | cat -n
echo "== FFmpeg types filter related =="
fd -a 'filters\.ts$|options\.ts$|spawn\.ts$' . -x sh -c 'echo "--- $1"; cat -n "$1"' sh {}
echo "== behavioral probe current/proposed escaping =="
python3 - <<'PY'
import re
def current_filter(lut, interp="trilinear"):
is_windows = False
sanitised = is_windows and lut or lut.replace("\\", "/")
escaped_lut = re.sub(r"(['\\,;\[\]%:])", r"\\\1", sanitised)
return f"lut3d='{escaped_lut}':interp={interp}"
def proposed_filter(lut, interp="trilinear"):
is_windows = False
sanitised = is_windows and lut or lut.replace("\\", "/")
escaped_lut = re.sub(r"\\", r"\\\\", sanitised)
escaped_lut = re.sub(r"'", r"'\''", escaped_lut)
escaped_lut = re.sub(r"([,;\[\]%:])", r"\\\1", escaped_lut)
return f"lut3d='{escaped_lut}':interp={interp}"
cases = [
"film.cube",
"/tmp/grade's.cube",
"/tmp/a;b.cube",
"/tmp/a\\b.cube",
"/tmp/trap'''.cube",
]
for case in cases:
print("lut =", repr(case))
print("current=", current_filter(case))
print("proposed=", proposed_filter(case))
PYRepository: GlobalTechInfo/mediaforge
Length of output: 17994
🌐 Web query:
FFmpeg filtergraph string syntax escape quote character close quote escape reopen quote 'example
💡 Result:
In FFmpeg, the quote character (') cannot be quoted or escaped while inside a quoted string [1]. To include a literal quote character within a string that is already being quoted, you must close the current quote, escape the quote character, and then reopen the quote [1]. The correct sequence to represent a literal quote is: '''. For example, to include the string Crime d'Amour in a context that requires quoting, you would write: 'Crime d'''Amour' [1]. Because FFmpeg filtergraphs often require multiple layers of escaping—specifically, one level for the filter option value, a second for the filtergraph description itself, and a third for the shell command line—managing these characters can become complex [2][1][3]. Escaping levels summary: 1. First Level: Escapes content of the filter option (e.g., escaping ':' or ''' inside an option value) [2][3]. 2. Second Level: Escapes the whole filter description (e.g., escaping '[],;' used by the filtergraph) [2][3]. 3. Third Level: Escapes shell-specific special characters when passed via the command line (e.g., escaping '' for the shell) [2][3]. Due to this complexity, FFmpeg documentation advises that for complex filter strings, it is often safer to read the filtergraph from an external file rather than including it directly in the shell command [4][5]. Additionally, the tool tools/ffescape can be used to automatically handle these escaping requirements [1].
Citations:
- 1: https://ffmpeg.org/ffmpeg-utils.html
- 2: https://ffmpeg.org/ffmpeg-filters.html
- 3: https://manpages.ubuntu.com/manpages/noble/man1/ffmpeg-filters.1.html
- 4: https://github.com/FFmpeg/FFmpeg/blob/125bb2e0/doc/filters.texi
- 5: https://ffmpeg.org/ffmpeg-filters.html?drawtext-1=
Escape the LUT path for FFmpeg filter option syntax.
lut3d= uses FFmpeg single-quote escaping; an apostrophe in the LUT path closes the outer quote and can make the filter fail to parse or accept injected filter syntax. Escape the path before wrapping it in lut3d='...'.
Use FFmpeg-compatible apostrophe escaping. For example, close the quote, escape the apostrophe, and reopen it instead of emitting \' inside the outer single quotes.
🤖 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 `@lib/helpers/edit.ts` around lines 611 - 612, Update the LUT path escaping in
the helper that builds the `lut3d` filter so apostrophes use FFmpeg-compatible
quote-close, escaped-apostrophe, and quote-reopen syntax rather than `\'` inside
the outer quotes. Preserve the existing escaping for the other filter-option
characters and continue wrapping the resulting path in `lut3d='...'`.
| const ext = path.extname(filename) || '.png'; | ||
| const base = path.basename(filename, ext); | ||
| const outName = base.replace(/%\d*\.?\d*[diouxX]/g, String(i + 1).padStart(4, '0')) + ext; | ||
| const outName = base.replace(/%\d*(?:\.\d*)?[diouxX]/g, String(i + 1).padStart(4, '0')) + ext; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 5 '\b(executeTimestamps|buildTimestampFilename)\b' --glob '*.ts' .
rg -n -C 2 '%04\.2d|%[0-9]*[diouxX]' --glob '*.ts' .Repository: GlobalTechInfo/mediaforge
Length of output: 30757
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Inspect the relevant helper implementation and export/import usage.
sed -n '1,220p' lib/helpers/screenshots.ts | nl -ba
# Programmatically compare the two regexes against representative specifiers.
node - <<'JS'
const executeRegex = /%\d*(?:\.\d*)?[diouxX]/g;
const buildRegex = /%0?\d*d/;
const samples = [
'%d', '%04d', '%05d', '%04.2d', '%.2d', '%04i', '%o', '%x', '%X',
'%04.2i', '%.3d', '%04U', '%04.2U',
];
for (const s of samples) {
const executeResult = s.replace(executeRegex, String(1).padStart(4, '0'));
const buildResult = s.replace(buildRegex, String(1).padStart(4, '0'));
console.log(`${s.padEnd(8)} execute=${executeResult.padEnd(10)} build=${buildResult.padEnd(10)} diff=${executeResult !== buildResult}`);
}
JSRepository: GlobalTechInfo/mediaforge
Length of output: 203
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Inspect the relevant helper implementation and export/import usage.
sed -n '1,220p' lib/helpers/screenshots.ts | awk '{printf "%5d\t%s\n", NR, $0}'
# Programmatically compare the two regexes against representative specifiers.
node - <<'JS'
const executeRegex = /%\d*(?:\.\d*)?[diouxX]/g;
const buildRegex = /%0?\d*d/;
const samples = [
'%d', '%04d', '%05d', '%04.2d', '%.2d', '%04i', '%o', '%x', '%X',
'%04.2i', '%.3d', '%04U', '%04.2U',
];
for (const s of samples) {
const executeResult = s.replace(executeRegex, String(1).padStart(4, '0'));
const buildResult = s.replace(buildRegex, String(1).padStart(4, '0'));
console.log(`${s.padEnd(8)} execute=${executeResult.padEnd(10)} build=${buildResult.padEnd(10)} diff=${executeResult !== buildResult}`);
}
JSRepository: GlobalTechInfo/mediaforge
Length of output: 8993
Keep buildTimestampFilename and executeTimestamps on the same filename format.
executeTimestamps accepts broader specifiers like %04.2d or %04i, but buildTimestampFilename only handles %0?\d*d. A caller can pass pattern screenshot_%04.2d.png to both helpers and get different output paths. Share one filename builder/validator, or restrict executeTimestamps to the supported format.
🤖 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 `@lib/helpers/screenshots.ts` at line 59, Align the filename-format handling
between buildTimestampFilename and executeTimestamps: ensure both accept and
expand the same timestamp specifiers, including formats such as %04.2d and %04i,
or restrict executeTimestamps to buildTimestampFilename’s supported syntax.
Reuse a shared filename builder or validator so identical patterns always
produce identical paths.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1 participant
Resolves all 11 open code scanning alerts:
GitHub Actions (7) —
actions/code-injection,actions/untrusted-checkout:github.event.workflow_run.head_branchinto shellrunsteps inpublish.yml,jsr.yml,docs.yml. Usehead_shapassed via an env var and gate the checkout on the run originating from the same repository.JavaScript (4):
js/shell-command-injection-from-environment—battle.test.ts,deno-tests/battle.test.ts: useexecFileSyncwith an argument array instead of a shell command string.js/incomplete-sanitization—lib/helpers/edit.ts: escape all ffmpeg filtergraph metacharacters (\ ' , ; [ ] % :) inapplyLUT.js/polynomial-redos—lib/helpers/screenshots.ts: rewrite the filename pattern%\\d*(?:\\.\\d*)?[diouxX]to remove nested quantifiers (behavior verified identical over 20k random strings).Local verification:
npm run build,npm run typecheck, 1187 unit/integration tests, node battle (557) and deno battle (550) all pass.Summary by CodeRabbit
Bug Fixes
Reliability