-
Notifications
You must be signed in to change notification settings - Fork 1
fix(security): resolve all 11 code scanning alerts #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -608,7 +608,7 @@ export async function applyLUT(opts: ApplyLutOptions): Promise<void> { | |
|
|
||
| const isWindows = process.platform === 'win32'; | ||
| const sanitised = isWindows ? lut : lut.replace(/\\/g, '/'); | ||
| const escapedLut = sanitised.replace(/:/g, '\\:').replace(/'/g, "\\'"); | ||
| const escapedLut = sanitised.replace(/(['\\,;\[\]%:])/g, '\\$1'); | ||
| const filter = `lut3d='${escapedLut}':interp=${interp}`; | ||
|
Comment on lines
+611
to
612
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 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:
💡 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 Citations:
🏁 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:
💡 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:
Escape the LUT path for FFmpeg filter option syntax.
Use FFmpeg-compatible apostrophe escaping. For example, close the quote, escape the apostrophe, and reopen it instead of emitting 🤖 Prompt for AI Agents |
||
|
|
||
| const builder = new FFmpegBuilder(input).setBinary(binary) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,7 +56,7 @@ async function executeTimestamps(opts: ScreenshotOptions, timestamps: (string | | |
| const ts = timestamps[i]!; | ||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ 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
🤖 Prompt for AI Agents |
||
| const outPath = path.join(folder, outName); | ||
|
|
||
| const args: string[] = ['-y', '-ss', String(toSeconds(ts)), '-i', input, '-vframes', '1']; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: GlobalTechInfo/mediaforge
Length of output: 50382
Replace the string-based FFmpeg wrappers with array APIs.
execFileSyncdoes 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. ChangeffmpegExecin bothbattle.test.tsanddeno-tests/battle.test.tsto acceptreadonly string[]and forward['-y', ...args]directly toexecFileSync, 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