Skip to content

Commit 0dc608d

Browse files
committed
fix(code-placeholders): guard shell values against arithmetic subscript execution
1 parent 14e6c0a commit 0dc608d

3 files changed

Lines changed: 108 additions & 29 deletions

File tree

‎apps/sim/lib/execution/code-placeholders/compiler.test.ts‎

Lines changed: 70 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1053,48 +1053,107 @@ describe('code placeholder compiler', () => {
10531053
'total=$(( $(( 1 + 1 )) + {{KEY}} ))',
10541054
'cat <<PAYLOAD\n$(( {{KEY}} * 2 ))\nPAYLOAD',
10551055
'cat <<PAYLOAD\n$[ {{KEY}} * 2 ]\nPAYLOAD',
1056-
])('rejects shell placeholders whose values enter arithmetic: %s', async (code) => {
1056+
// Rejection is value-based and position-independent: a value whose array subscript carries an
1057+
// expansion would run the embedded command only if it reached arithmetic, so it is refused
1058+
// wherever the placeholder lands — including ordinary, non-arithmetic positions.
1059+
'printf "%s" "{{KEY}}"',
1060+
'[ "{{KEY}}" = x ]',
1061+
'echo {{KEY}}',
1062+
'cat <<PAYLOAD\n{{KEY}}\nPAYLOAD',
1063+
"cat <<'PAYLOAD'\n{{KEY}}\nPAYLOAD",
1064+
])(
1065+
'rejects a shell value whose array subscript contains an expansion, in any position: %s',
1066+
async (code) => {
1067+
await expect(
1068+
compileCodePlaceholders({
1069+
code,
1070+
language: CodeLanguage.Shell,
1071+
environmentVariables: { KEY: 'values[$(printf injected >&2)]' },
1072+
})
1073+
).rejects.toThrow('array subscript with an expansion')
1074+
}
1075+
)
1076+
1077+
it.each([
1078+
'values[`printf injected >&2`]',
1079+
'a[$(id)]',
1080+
'm[$(cmd)]=x',
1081+
'a[<(cmd)]',
1082+
// biome-ignore lint/suspicious/noTemplateCurlyInString: shell expansion inside a subscript, not a JS template
1083+
'x[${!ref}]',
1084+
// biome-ignore lint/suspicious/noTemplateCurlyInString: shell expansion inside a subscript, not a JS template
1085+
'x[${v@P}]',
1086+
])('rejects every expansion flavour inside a shell value subscript: %s', async (value) => {
10571087
await expect(
10581088
compileCodePlaceholders({
1059-
code,
1089+
code: 'printf "%s" "{{KEY}}"',
10601090
language: CodeLanguage.Shell,
1061-
environmentVariables: { KEY: 'values[$(printf injected >&2)]' },
1091+
environmentVariables: { KEY: value },
10621092
})
1063-
).rejects.toThrow('is not supported in shell arithmetic')
1093+
).rejects.toThrow('array subscript with an expansion')
10641094
})
10651095

1066-
it('preserves shell literal arithmetic text and leaves completed arithmetic frames', async () => {
1067-
const value = 'values[$(printf injected >&2)]'
1096+
it('preserves shell literal arithmetic text for values without a subscript expansion', async () => {
1097+
const value = 'raw $(text) and `more` [0] value'
10681098
const compiled = await compileCodePlaceholders({
10691099
code: [
10701100
'printf "%s\\n" "{{KEY}}"',
10711101
"printf '%s\\n' '$(( {{KEY}} ))'",
10721102
"printf '%s\\n' '$[ {{KEY}} ]'",
10731103
'printf "%s\\n" "$(printf %s "{{KEY}}")"',
10741104
'printf "%s\\n" "$(( 1 + 1 )){{KEY}}"',
1075-
'printf "%s\\n" "$[ values[0] + 2 ]{{KEY}}"',
10761105
'(( total = 2 )); printf "%s\\n" "{{KEY}}"',
10771106
'cat <<PAYLOAD',
1078-
'(( {{KEY}} ))',
1107+
'{{KEY}}',
10791108
'PAYLOAD',
10801109
].join('\n'),
10811110
language: CodeLanguage.Shell,
10821111
environmentVariables: { KEY: value },
10831112
})
10841113

10851114
expect(executeShell(compiled.code, compiled.bindings)).toBe(
1086-
`${value}\n$(( ${value} ))\n$[ ${value} ]\n${value}\n2${value}\n2${value}\n${value}\n(( ${value} ))\n`
1115+
`${value}\n$(( ${value} ))\n$[ ${value} ]\n${value}\n2${value}\n${value}\n${value}\n`
10871116
)
10881117
})
10891118

1090-
it('discovers shell arithmetic placeholders without compiling missing values', async () => {
1119+
it.each([
1120+
'total=$(( {{KEY}} * 2 ))',
1121+
'(( total = {{KEY}} ))',
1122+
'total=$[ {{KEY}} ]',
1123+
'[[ {{KEY}} -eq 0 ]]',
1124+
'let "x={{KEY}}"',
1125+
'declare -i x="{{KEY}}"',
1126+
// biome-ignore lint/suspicious/noTemplateCurlyInString: shell parameter expansion, not a JS template
1127+
's=abcdef; printf "%s\\n" "${s:{{KEY}}}"',
1128+
// biome-ignore lint/suspicious/noTemplateCurlyInString: shell parameter expansion, not a JS template
1129+
'a=(1 2); printf "%s\\n" "${a[{{KEY}}]}"',
1130+
// biome-ignore lint/suspicious/noTemplateCurlyInString: shell parameter expansion, not a JS template
1131+
'a=([{{KEY}}]=text); printf "%s\\n" "${a[*]}"',
1132+
'[[ "{{KEY}}" == "-eq" ]] && printf "%s\\n" matched',
1133+
'unset "a[{{KEY}}]"',
1134+
])('compiles a safe value in any position, including arithmetic ones: %s', async (code) => {
1135+
// A value with no array-subscript expansion cannot run a command in arithmetic (verified in
1136+
// bash 5.2), so it is accepted everywhere — no position detection, no over-rejection.
1137+
await expect(
1138+
compileCodePlaceholders({
1139+
code,
1140+
language: CodeLanguage.Shell,
1141+
environmentVariables: { KEY: 'items[0]' },
1142+
})
1143+
).resolves.toBeDefined()
1144+
})
1145+
1146+
it('discovers shell arithmetic placeholders without binding missing values', async () => {
10911147
const code = 'total=$(( {{MISSING}} + {{KEY}} ))'
10921148
await expect(analyzeCodePlaceholders(code, CodeLanguage.Shell)).resolves.toEqual([
10931149
'MISSING',
10941150
'KEY',
10951151
])
10961152
const compiled = await compileCodePlaceholders({ code, language: CodeLanguage.Shell })
1097-
expect(compiled.code).toBe(code)
1153+
// With no configured values nothing is bound; unresolved placeholders expand to empty, as in
1154+
// every other position (the arithmetic position itself is no longer special).
1155+
expect(compiled.bindings).toEqual([])
1156+
expect(compiled.code).toBe('total=$(( + ))')
10981157
})
10991158

11001159
it('renders quoted shell heredocs nested in double-quoted command substitutions', async () => {

‎apps/sim/lib/execution/code-placeholders/shared.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -327,12 +327,12 @@ export function createOffsetRangeLookup(
327327
* The placeholders lying wholly inside `[start, end]`. Occurrences are ordered and never
328328
* overlap, so the matches are one contiguous run found by bisection.
329329
*/
330-
export function occurrencesWithin(
331-
occurrences: readonly CodePlaceholderOccurrence[],
330+
export function occurrencesWithin<T extends Pick<CodePlaceholderOccurrence, 'start' | 'end'>>(
331+
occurrences: readonly T[],
332332
start: number,
333333
end: number
334-
): CodePlaceholderOccurrence[] {
335-
const matches: CodePlaceholderOccurrence[] = []
334+
): T[] {
335+
const matches: T[] = []
336336
for (
337337
let index = partitionPoint(occurrences, (occurrence) => occurrence.start < start);
338338
index < occurrences.length && occurrences[index].end <= end;

‎apps/sim/lib/execution/code-placeholders/shell.ts‎

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,33 @@ interface ShellScanFrame {
3737

3838
interface ShellOccurrenceContext {
3939
quote: ShellQuote
40-
/** Includes nested command substitutions whose output can become an arithmetic operand. */
41-
arithmetic?: boolean
4240
unsupported?: 'escaped sequence'
4341
}
4442

43+
/**
44+
* Whether a resolved value would run a command if it reached shell arithmetic evaluation. The only
45+
* such path is an array subscript whose expression contains an expansion: when bash evaluates
46+
* `name[expr]` as arithmetic it expands `expr`, so a command substitution (`$(…)`, backticks) or a
47+
* process substitution (`<(…)`, `>(…)`) there executes, and prompt/indirection expansions can too.
48+
* Plain `$(…)` text, `${…}` and `$((…))` outside a subscript are never executed by arithmetic, so
49+
* only the bracketed case is refused. The depth walk is conservative: ANY `$`, backtick, or process
50+
* substitution inside `[ … ]` is rejected (e.g. `a[$((1+1))]`), which is safe to over-refuse in data.
51+
*/
52+
function hasArithmeticExecutingSubscript(value: string): boolean {
53+
let bracketDepth = 0
54+
for (let index = 0; index < value.length; index += 1) {
55+
const character = value[index]
56+
if (character === '[') bracketDepth += 1
57+
else if (character === ']') {
58+
if (bracketDepth > 0) bracketDepth -= 1
59+
} else if (bracketDepth > 0) {
60+
if (character === '$' || character === '`') return true
61+
if ((character === '<' || character === '>') && value[index + 1] === '(') return true
62+
}
63+
}
64+
return false
65+
}
66+
4567
function lineEndAfterNewline(code: string, start: number): number {
4668
const newline = code.indexOf('\n', start)
4769
return newline === -1 ? code.length : newline + 1
@@ -450,7 +472,6 @@ function getUnsupportedShellPosition(
450472
occurrence: CodePlaceholderOccurrence,
451473
context: ShellOccurrenceContext
452474
): string | undefined {
453-
if (context.arithmetic) return 'in shell arithmetic'
454475
if (code[occurrence.start - 1] === '$') return 'immediately after "$"'
455476
if (context.quote !== 'none') return undefined
456477

@@ -492,7 +513,6 @@ function collectShellOccurrenceContexts(
492513
{ kind: 'root', quote: 'none', parenthesisDepth: 0, literalRoot },
493514
]
494515
let skippedRangeIndex = 0
495-
let arithmeticDepth = 0
496516

497517
for (let index = start; index < end; ) {
498518
const frame = frames.at(-1)
@@ -512,7 +532,7 @@ function collectShellOccurrenceContexts(
512532

513533
const occurrence = occurrenceByStart.get(index)
514534
if (occurrence) {
515-
contexts.set(occurrence, { quote: frame.quote, arithmetic: arithmeticDepth > 0 })
535+
contexts.set(occurrence, { quote: frame.quote })
516536
index = occurrence.end
517537
continue
518538
}
@@ -552,7 +572,6 @@ function collectShellOccurrenceContexts(
552572
...(brackets ? { bracketDepth: 1 } : {}),
553573
literalRoot: false,
554574
})
555-
arithmeticDepth += 1
556575
index += arithmeticExpansion && !brackets ? 3 : 2
557576
continue
558577
}
@@ -649,10 +668,7 @@ function collectShellOccurrenceContexts(
649668
if (character === '[') frame.bracketDepth += 1
650669
if (character === ']') {
651670
frame.bracketDepth -= 1
652-
if (frame.bracketDepth === 0) {
653-
frames.pop()
654-
arithmeticDepth -= 1
655-
}
671+
if (frame.bracketDepth === 0) frames.pop()
656672
}
657673
index += 1
658674
continue
@@ -664,10 +680,7 @@ function collectShellOccurrenceContexts(
664680
}
665681
if ((frame.kind === 'command' || frame.kind === 'arithmetic') && character === ')') {
666682
frame.parenthesisDepth -= 1
667-
if (frame.parenthesisDepth === 0) {
668-
frames.pop()
669-
if (frame.kind === 'arithmetic') arithmeticDepth -= 1
670-
}
683+
if (frame.parenthesisDepth === 0) frames.pop()
671684
index += 1
672685
continue
673686
}
@@ -782,6 +795,13 @@ export async function compileShellPlaceholders(
782795
occurrence.start
783796
)
784797
}
798+
if (resolved && !input.analysisOnly && hasArithmeticExecutingSubscript(resolved.value)) {
799+
throw new CodePlaceholderCompileError(
800+
`Variable placeholder "${occurrence.name}" cannot contain an array subscript with an expansion in shell code`,
801+
input.code,
802+
occurrence.start
803+
)
804+
}
785805
return resolved
786806
}
787807
const resolveShellOccurrence = (occurrence: CodePlaceholderOccurrence) =>

0 commit comments

Comments
 (0)