fix(llm-security): YAML/workflow parser divergence — block scalars + bare if: (#32,#33,#43)
#33 the frontmatter parser collected a block-scalar body (description: |) but did not skip it, so an indented name:/allowed_tools: inside the body re-matched as a top-level key and overrode the real values TRG-shadow and permission checks depend on; the parser now consumes block-scalar bodies as opaque content. #32 block-scalar headers carrying indentation/chomping indicators (|2, >-, |-2) were not recognized, so their bodies never reached the run: injection sink; now matched via a proper indicator/chomping regex. #43 the B4 actor auth-bypass detector inspected only braced ${{ }} expressions, missing the canonical bare 'if: github.actor == ...' form (Synacktiv Dependabot-spoof false negative); bare if: expressions now emit a synthetic event the detector reads. Suite 2004/0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TcQyMTQfyrsAapaCMPxTtQ
This commit is contained in:
parent
207385fbbe
commit
b224e18b42
8 changed files with 244 additions and 7 deletions
|
|
@ -20,7 +20,10 @@ const EXPR_RE = /\$\{\{\s*([\s\S]+?)\s*\}\}/g;
|
|||
const KV_RE = /^([A-Za-z_][\w-]*)\s*:\s*(.*)$/;
|
||||
const LIST_KV_RE = /^-\s+([A-Za-z_][\w-]*)\s*:\s*(.*)$/;
|
||||
const TRIGGER_RE = /^([a-z_]+)(?::|$)/;
|
||||
const BLOCK_SCALAR_VALUES = new Set(['|', '>', '|-', '>-', '|+', '>+']);
|
||||
// Block-scalar header: `|` or `>` plus optional indentation indicator
|
||||
// (1-9) and chomping indicator (`+`/`-`), in either order (`|2`, `>-`,
|
||||
// `|-2`, `>2-`, ...). Bare-literal matching missed indicator forms (#32).
|
||||
const BLOCK_SCALAR_RE = /^[|>](?:[1-9][+-]?|[+-][1-9]?)?$/;
|
||||
|
||||
/**
|
||||
* Strip comments after first unquoted `#`. Workflows rarely embed `#`
|
||||
|
|
@ -131,6 +134,7 @@ export function extractTriggers(lines) {
|
|||
* parent: string,
|
||||
* parentChain: string[],
|
||||
* blockScalar: boolean,
|
||||
* bare?: boolean,
|
||||
* }[],
|
||||
* }}
|
||||
*/
|
||||
|
|
@ -177,7 +181,7 @@ export function parseWorkflow(text) {
|
|||
if (kv) {
|
||||
const key = kv[1];
|
||||
const value = kv[2];
|
||||
const isBlock = BLOCK_SCALAR_VALUES.has(value);
|
||||
const isBlock = BLOCK_SCALAR_RE.test(value);
|
||||
const exprs = !isBlock && value ? findExpressions(raw, i + 1) : [];
|
||||
for (const e of exprs) {
|
||||
events.push({
|
||||
|
|
@ -187,6 +191,20 @@ export function parseWorkflow(text) {
|
|||
blockScalar: false,
|
||||
});
|
||||
}
|
||||
// Bare `if:` values (no `${{ }}`) are auto-evaluated as expressions
|
||||
// by the runner — emit them so actor auth-bypass checks see the
|
||||
// canonical unbraced form (#43).
|
||||
if (key === 'if' && !isBlock && value && exprs.length === 0) {
|
||||
events.push({
|
||||
line: i + 1,
|
||||
column: raw.indexOf(value) + 1,
|
||||
expr: value.trim(),
|
||||
parent: key,
|
||||
parentChain: [...stack.map(s => s.key), key],
|
||||
blockScalar: false,
|
||||
bare: true,
|
||||
});
|
||||
}
|
||||
stack.push({ indent, key, isBlockScalar: isBlock });
|
||||
continue;
|
||||
}
|
||||
|
|
@ -196,7 +214,7 @@ export function parseWorkflow(text) {
|
|||
if (lkv) {
|
||||
const key = lkv[1];
|
||||
const value = lkv[2];
|
||||
const isBlock = BLOCK_SCALAR_VALUES.has(value);
|
||||
const isBlock = BLOCK_SCALAR_RE.test(value);
|
||||
const exprs = !isBlock && value ? findExpressions(raw, i + 1) : [];
|
||||
for (const e of exprs) {
|
||||
events.push({
|
||||
|
|
@ -206,6 +224,18 @@ export function parseWorkflow(text) {
|
|||
blockScalar: false,
|
||||
});
|
||||
}
|
||||
// Same bare `if:` handling as the KV branch (#43).
|
||||
if (key === 'if' && !isBlock && value && exprs.length === 0) {
|
||||
events.push({
|
||||
line: i + 1,
|
||||
column: raw.indexOf(value) + 1,
|
||||
expr: value.trim(),
|
||||
parent: key,
|
||||
parentChain: [...stack.map(s => s.key), key],
|
||||
blockScalar: false,
|
||||
bare: true,
|
||||
});
|
||||
}
|
||||
// List items create a deeper synthetic indent so subsequent
|
||||
// sibling keys at the same column still resolve to this item.
|
||||
stack.push({ indent: indent + 2, key, isBlockScalar: isBlock });
|
||||
|
|
|
|||
|
|
@ -19,7 +19,9 @@ export function parseFrontmatter(content) {
|
|||
const result = {};
|
||||
|
||||
// Parse simple key: value pairs
|
||||
for (const line of block.split('\n')) {
|
||||
const lines = block.split('\n');
|
||||
for (let lineIdx = 0; lineIdx < lines.length; lineIdx++) {
|
||||
const line = lines[lineIdx];
|
||||
const trimmed = line.trim();
|
||||
if (!trimmed || trimmed.startsWith('#')) continue;
|
||||
|
||||
|
|
@ -44,12 +46,13 @@ export function parseFrontmatter(content) {
|
|||
// Handle multi-line description with |
|
||||
if (value === '|' || value === '>') {
|
||||
const descLines = [];
|
||||
const lines = block.split('\n');
|
||||
const lineIdx = lines.indexOf(line);
|
||||
for (let i = lineIdx + 1; i < lines.length; i++) {
|
||||
const dLine = lines[i];
|
||||
if (/^\S/.test(dLine) && !dLine.startsWith(' ') && !dLine.startsWith('\t')) break;
|
||||
descLines.push(dLine.replace(/^ /, ''));
|
||||
// Consume the body line — block-scalar content is an opaque
|
||||
// string, never re-parsed as key: value pairs (#33).
|
||||
lineIdx = i;
|
||||
}
|
||||
value = descLines.join('\n').trim();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -274,7 +274,7 @@ async function scanFile(absPath, targetPath, stderrLog) {
|
|||
`${relPath}: ${ev.expr}.`,
|
||||
file: relPath,
|
||||
line: ev.line,
|
||||
evidence: `\${{ ${ev.expr} }}`,
|
||||
evidence: ev.bare ? ev.expr : `\${{ ${ev.expr} }}`,
|
||||
owasp: 'LLM06',
|
||||
recommendation:
|
||||
'Use `github.event.pull_request.user.login` (immutable per PR) ' +
|
||||
|
|
|
|||
14
tests/fixtures/workflows/.github/workflows/auth-bypass-bare-if.yml
vendored
Normal file
14
tests/fixtures/workflows/.github/workflows/auth-bypass-bare-if.yml
vendored
Normal file
|
|
@ -0,0 +1,14 @@
|
|||
name: dependabot trust check (bare if — Synacktiv 2023)
|
||||
on:
|
||||
pull_request_target:
|
||||
branches: [main]
|
||||
|
||||
jobs:
|
||||
auto-merge:
|
||||
runs-on: ubuntu-latest
|
||||
if: github.actor == 'dependabot[bot]'
|
||||
steps:
|
||||
- name: Checkout PR
|
||||
uses: actions/checkout@v4
|
||||
- name: Approve
|
||||
run: gh pr review --approve
|
||||
13
tests/fixtures/workflows/.github/workflows/tp-block-scalar-indent.yml
vendored
Normal file
13
tests/fixtures/workflows/.github/workflows/tp-block-scalar-indent.yml
vendored
Normal file
|
|
@ -0,0 +1,13 @@
|
|||
name: issue triage (block scalar with indentation indicator)
|
||||
on:
|
||||
pull_request_target:
|
||||
branches: [main]
|
||||
|
||||
jobs:
|
||||
triage:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- name: Echo title
|
||||
run: |2
|
||||
echo "Issue title:"
|
||||
echo "${{ github.event.issue.title }}"
|
||||
|
|
@ -101,6 +101,74 @@ describe('parseWorkflow — block scalars', () => {
|
|||
const { events } = parseWorkflow(yml);
|
||||
assert.ok(events.find(e => e.parent === 'run' && e.blockScalar));
|
||||
});
|
||||
|
||||
it('recognizes an indentation indicator (`run: |2`) as a block-scalar header (#32)', () => {
|
||||
const yml = [
|
||||
'on: pull_request_target',
|
||||
'jobs:',
|
||||
' j:',
|
||||
' steps:',
|
||||
' - name: indented',
|
||||
' run: |2',
|
||||
' echo "${{ github.event.issue.title }}"',
|
||||
].join('\n');
|
||||
const { events } = parseWorkflow(yml);
|
||||
const runs = events.filter(e => e.parent === 'run');
|
||||
assert.equal(runs.length, 1);
|
||||
assert.equal(runs[0].expr, 'github.event.issue.title');
|
||||
assert.equal(runs[0].blockScalar, true);
|
||||
});
|
||||
|
||||
it('recognizes combined indicator + chomping (`run: >2-`, `run: |-2`) (#32)', () => {
|
||||
for (const header of ['>2-', '|-2', '|2+']) {
|
||||
const yml = [
|
||||
'on: pull_request_target',
|
||||
'jobs:',
|
||||
' j:',
|
||||
' steps:',
|
||||
` - run: ${header}`,
|
||||
' echo "${{ github.head_ref }}"',
|
||||
].join('\n');
|
||||
const { events } = parseWorkflow(yml);
|
||||
const runs = events.filter(e => e.parent === 'run');
|
||||
assert.equal(runs.length, 1, `header ${header}: expected 1 run event, got ${runs.length}`);
|
||||
assert.equal(runs[0].blockScalar, true, `header ${header}: expected blockScalar=true`);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('parseWorkflow — bare if: values (#43)', () => {
|
||||
it('emits an if-context event for a bare (non-braced) if: expression', () => {
|
||||
const yml = [
|
||||
'on: pull_request_target',
|
||||
'jobs:',
|
||||
' j:',
|
||||
' runs-on: ubuntu-latest',
|
||||
" if: github.actor == 'dependabot[bot]'",
|
||||
' steps:',
|
||||
' - run: echo ok',
|
||||
].join('\n');
|
||||
const { events } = parseWorkflow(yml);
|
||||
const ifs = events.filter(e => e.parent === 'if');
|
||||
assert.equal(ifs.length, 1);
|
||||
assert.equal(ifs[0].expr, "github.actor == 'dependabot[bot]'");
|
||||
assert.equal(ifs[0].bare, true);
|
||||
assert.equal(ifs[0].line, 5);
|
||||
});
|
||||
|
||||
it('does not double-emit for braced if: expressions', () => {
|
||||
const yml = [
|
||||
'on: pull_request_target',
|
||||
'jobs:',
|
||||
' j:',
|
||||
" if: ${{ github.actor == 'dependabot[bot]' }}",
|
||||
' runs-on: ubuntu-latest',
|
||||
].join('\n');
|
||||
const { events } = parseWorkflow(yml);
|
||||
const ifs = events.filter(e => e.parent === 'if');
|
||||
assert.equal(ifs.length, 1);
|
||||
assert.ok(!ifs[0].bare);
|
||||
});
|
||||
});
|
||||
|
||||
describe('parseWorkflow — sink-mismatch contexts', () => {
|
||||
|
|
|
|||
90
tests/lib/yaml-frontmatter.test.mjs
Normal file
90
tests/lib/yaml-frontmatter.test.mjs
Normal file
|
|
@ -0,0 +1,90 @@
|
|||
// yaml-frontmatter.test.mjs — unit tests for the regex-based frontmatter
|
||||
// parser (scanners/lib/yaml-frontmatter.mjs).
|
||||
|
||||
import { describe, it } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
|
||||
const { parseFrontmatter } = await import('../../scanners/lib/yaml-frontmatter.mjs');
|
||||
|
||||
describe('parseFrontmatter — basics', () => {
|
||||
it('parses simple key: value pairs', () => {
|
||||
const content = [
|
||||
'---',
|
||||
'name: my-command',
|
||||
'description: does a thing',
|
||||
'model: opus',
|
||||
'---',
|
||||
'# Body',
|
||||
].join('\n');
|
||||
const fm = parseFrontmatter(content);
|
||||
assert.equal(fm.name, 'my-command');
|
||||
assert.equal(fm.description, 'does a thing');
|
||||
assert.equal(fm.model, 'opus');
|
||||
});
|
||||
|
||||
it('collects a block-scalar description body', () => {
|
||||
const content = [
|
||||
'---',
|
||||
'name: my-command',
|
||||
'description: |',
|
||||
' Line one.',
|
||||
' Line two.',
|
||||
'---',
|
||||
].join('\n');
|
||||
const fm = parseFrontmatter(content);
|
||||
assert.equal(fm.description, 'Line one.\nLine two.');
|
||||
});
|
||||
|
||||
it('returns null when no frontmatter is present', () => {
|
||||
assert.equal(parseFrontmatter('# Just markdown\n'), null);
|
||||
});
|
||||
});
|
||||
|
||||
describe('parseFrontmatter — block-scalar body is opaque (#33)', () => {
|
||||
it('does not let key-shaped lines inside a description: | body override real keys', () => {
|
||||
const content = [
|
||||
'---',
|
||||
'name: real-command',
|
||||
'description: |',
|
||||
' This helper is safe.',
|
||||
' name: evil-command',
|
||||
' allowed_tools: Bash(rm -rf *)',
|
||||
'model: opus',
|
||||
'---',
|
||||
'# Body',
|
||||
].join('\n');
|
||||
const fm = parseFrontmatter(content);
|
||||
assert.equal(fm.name, 'real-command');
|
||||
assert.equal(fm.allowed_tools, undefined);
|
||||
assert.ok(fm.description.includes('name: evil-command'));
|
||||
});
|
||||
|
||||
it('still parses keys that follow the block-scalar body', () => {
|
||||
const content = [
|
||||
'---',
|
||||
'description: |',
|
||||
' Indented body.',
|
||||
' name: shadow',
|
||||
'model: opus',
|
||||
'name: after-block',
|
||||
'---',
|
||||
].join('\n');
|
||||
const fm = parseFrontmatter(content);
|
||||
assert.equal(fm.model, 'opus');
|
||||
assert.equal(fm.name, 'after-block');
|
||||
});
|
||||
|
||||
it('treats folded-scalar (>) bodies as opaque too', () => {
|
||||
const content = [
|
||||
'---',
|
||||
'name: real-command',
|
||||
'description: >',
|
||||
' folded text',
|
||||
' allowed_tools: Bash',
|
||||
'---',
|
||||
].join('\n');
|
||||
const fm = parseFrontmatter(content);
|
||||
assert.equal(fm.name, 'real-command');
|
||||
assert.equal(fm.allowed_tools, undefined);
|
||||
});
|
||||
});
|
||||
|
|
@ -63,6 +63,13 @@ describe('workflow-scanner — true-positive cases', () => {
|
|||
assert.equal(fs[0].severity, 'high');
|
||||
assert.match(fs[0].evidence, /issue\.body/);
|
||||
});
|
||||
|
||||
it('flags issue.title inside `run: |2` (indentation indicator) as HIGH (#32)', () => {
|
||||
const fs = findingsByFile(result.findings, 'tp-block-scalar-indent.yml');
|
||||
assert.equal(fs.length, 1);
|
||||
assert.equal(fs[0].severity, 'high');
|
||||
assert.match(fs[0].evidence, /issue\.title/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('workflow-scanner — false-positive suppression', () => {
|
||||
|
|
@ -184,6 +191,18 @@ describe('workflow-scanner — B4 auth-bypass', () => {
|
|||
assert.match(auth.recommendation, /pull_request\.user\.login/);
|
||||
});
|
||||
|
||||
it('flags bare `if: github.actor == \'dependabot[bot]\'` (no ${{ }}) as MEDIUM auth-bypass (#43)', async () => {
|
||||
resetCounter();
|
||||
const r = await scan(FIXTURE_DIR);
|
||||
const fs = findingsByFile(r.findings, 'auth-bypass-bare-if.yml');
|
||||
const auth = fs.find(f => /Actor auth-bypass/i.test(f.title));
|
||||
assert.ok(auth, `expected an Actor auth-bypass finding in ${JSON.stringify(fs)}`);
|
||||
assert.equal(auth.severity, 'medium');
|
||||
assert.equal(auth.owasp, 'LLM06');
|
||||
assert.match(auth.evidence, /github\.actor == 'dependabot\[bot\]'/);
|
||||
assert.match(auth.recommendation, /pull_request\.user\.login/);
|
||||
});
|
||||
|
||||
it('does NOT flag plain `if: ${{ startsWith(github.head_ref, …) }}` as auth-bypass', async () => {
|
||||
resetCounter();
|
||||
const r = await scan(FIXTURE_DIR);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue