From 91e1b85cceef3476837bc1df355b1d55c9134e99 Mon Sep 17 00:00:00 2001 From: Joe Harris Date: Fri, 9 Oct 2026 10:30:03 -0400 Subject: [PATCH 01/17] Extend comment scanner with piped-stdin, runner-string, MDX, and worktree TypeScript support Teach the shell scanner to route echo/printf payloads piped into interpreters to the matching language parser, failing closed on dynamic producers and bare unmodeled-interpreter stdin sinks. Decompose single-string ssh, watch, xargs, env, sudo, and nohup arguments through a RUNNER_COMMANDS table with nested shell parsing, on both the shell and Python sides. Scan .mdx files with a dedicated handler covering prose fences, JSX comment expressions, and top-level import/export blocks. Resolve the TypeScript package from COMMENT_POLICY_TYPESCRIPT, the local results-explorer install, or the primary clone through git-common-dir so linked worktrees run the JavaScript scanner tests without extra setup. --- docs/internal/comment-policy.md | 12 +- scripts/comment_execution.py | 20 +- scripts/comment_payloads.py | 386 +++++++++++++++++++++- scripts/comment_syntax.py | 49 ++- scripts/comment_syntax_js.cjs | 15 +- tests/unit/scripts/test_comment_policy.py | 245 +++++++++++++- 6 files changed, 698 insertions(+), 29 deletions(-) diff --git a/docs/internal/comment-policy.md b/docs/internal/comment-policy.md index dcac7876f2..789dd51ebb 100644 --- a/docs/internal/comment-policy.md +++ b/docs/internal/comment-policy.md @@ -172,8 +172,8 @@ Known gaps, each a place where a comment can pass unreported: - Unknown programs that take `-c` or `-e` as data are listed in `DATA_FLAG_PROGRAMS` in `scripts/comment_execution.py`; any other program given such a flag is reported. -- Commands passed to a runner such as `ssh` or `watch` as one string are not - split, so an interpreter inside that string is not followed. +- Commands passed to a runner such as `ssh` or `watch` as a dynamic string + are not split, so an interpreter inside that string is not followed. - HTML comments and MyST `%` lines in Markdown prose are not scanned. In the maintained docs they are copyright headers, generator start and end markers, `` markers read by `scripts/blog_content_validation.py`, @@ -183,10 +183,10 @@ Known gaps, each a place where a comment can pass unreported: such comments where they are part of the product's output, for example maintenance SQL headers, dry-run DDL previews, stream-file headers, the Trino tuning note and the AWS Glue job script. -- A program piped to an unmodeled interpreter on stdin (such as `printf '...' | perl` - or `echo '...' | ruby`) is not scanned. +- A program piped to an interpreter on stdin from a producer other than `echo` + or `printf` (such as `curl ... | sh`) is not scanned. - A whole-file pinned notice in `quality/comment-cleanup-scope.json` (such as `_sources/tpc-h/PATCHES.md`) is verified against its recorded digest, but changes to the file are not guarded to require an accompanying pin update on branches. -- MDX (`.mdx`) files map to the Markdown scanner, so MDX imports and JSX `{/* */}` - comments outside Markdown code fences are not inspected. +- JSX `{// ...}` line-comment expressions in MDX prose are not extracted, so + comments in that form outside Markdown code fences are not inspected. diff --git a/scripts/comment_execution.py b/scripts/comment_execution.py index c946edf0d0..821dc22c90 100644 --- a/scripts/comment_execution.py +++ b/scripts/comment_execution.py @@ -5,10 +5,12 @@ from collections import defaultdict from comment_payloads import ( + RUNNER_COMMANDS, SQL_CLIENTS, command_words, inline_source_index, runner_command_start, + runner_trailing_index, sql_client_source_index, ) @@ -650,7 +652,7 @@ def runner_payload( except ValueError: return args[0], "unsupported", None for index, word in enumerate(words[first:], start=first): - nested = word.rsplit("/", 1)[-1] if word else None + nested = word.rsplit("/", 1)[-1] if word and not any(char.isspace() for char in word) else None if nested and ( nested.startswith("python") or nested in {"node", "sh", "bash", "zsh"} | SQL_CLIENTS | UNMODELED_INTERPRETERS @@ -660,8 +662,24 @@ def runner_payload( word is not None and INLINE_SHAPED_FLAG.fullmatch(word) for word in words[1:] ): return args[0], "unsupported", None + if name in RUNNER_COMMANDS: + inner = self.runner_string_args(args, words, name) + if inner is not None: + return inner return None + def runner_string_args( + self, args: list[ast.expr], words: list[str | None], name: str + ) -> tuple[ast.AST, str, str | None] | None: + first = runner_trailing_index(words, name) + trailing = words[first:] + if len(trailing) != 1 or trailing[0] is None: + return None + text = self.literal(args[first]) + if text is None or not text.strip(): + return None + return args[first], "bash", text + def inline_process_payload( self, args: list[ast.expr], words: list[str | None], language: str | None ) -> tuple[ast.AST, str, str | None] | None: diff --git a/scripts/comment_payloads.py b/scripts/comment_payloads.py index 0ce2e9294d..333bb85fa0 100644 --- a/scripts/comment_payloads.py +++ b/scripts/comment_payloads.py @@ -321,6 +321,84 @@ def astro_expression_sources(path: str, source: str) -> list[tuple[int, str, str ] +MDX_JSX_COMMENT = re.compile(r"\{/\*.*?\*/\s*\}", re.S) +MDX_STATEMENT_START = re.compile(r"(?:import|export)(?=\s|$)") +MDX_IMPORT_FROM = re.compile(r"\bfrom\s+['\"][^'\"\n]+['\"]") +MDX_IMPORT_BARE = re.compile(r"^import\s+['\"]") +MDX_EXPORT_SHAPE = re.compile(r"^export\s+(?:default|const|let|var|function|class|async|\{|\*)") +MDX_STATEMENT_CONTINUE = re.compile(r"(?:[{(\[=,:+*/?&|>-]|\bfrom|\bdefault)\s*$") + + +def mdx_masked_lines(source: str) -> list[str | None]: + lines: list[str | None] = list(source.splitlines()) + for token in MarkdownIt("commonmark").parse(source): + if token.type in {"fence", "code"} and token.map is not None: + for number in range(token.map[0], token.map[1]): + lines[number] = None + return lines + + +def mdx_statement_end(lines: list[str | None], start: int) -> int: + depth = 0 + index = start + while index < len(lines): + line = lines[index] + if line is None: + raise ValueError("mdx code fence inside JavaScript statement requires an adapter") + cursor = 0 + while cursor < len(line): + char = line[cursor] + if char in "'\"`": + closing = line.find(char, cursor + 1) + while closing >= 0 and line[closing - 1] == "\\": + closing = line.find(char, closing + 1) + if closing < 0: + raise ValueError("unterminated string in MDX JavaScript statement") + cursor = closing + 1 + continue + depth += (char in "{([") - (char in "})]") + cursor += 1 + index += 1 + if depth <= 0 and (index - start > 1 or not MDX_STATEMENT_CONTINUE.search(line)): + break + if depth > 0: + raise ValueError("unterminated JavaScript statement in MDX source") + return index + + +def mdx_js_sources(path: str, source: str) -> list[tuple[int, str, str, str, str]]: + lines = mdx_masked_lines(source) + result = [] + index = 0 + while index < len(lines): + line = lines[index] + if line is not None and MDX_STATEMENT_START.match(line): + end = mdx_statement_end(lines, index) + text = "\n".join(str(lines[number]) for number in range(index, end)) + keep = ( + MDX_IMPORT_FROM.search(text) or MDX_IMPORT_BARE.match(text) + if line.startswith("import") + else MDX_EXPORT_SHAPE.match(text) + ) + if keep: + result.append((index + 1, path + ".tsx", text, "javascript", f"mdx:{len(result)}")) + index = end + else: + index += 1 + return result + + +def mdx_jsx_comments(source: str) -> list[tuple[int, str]]: + lines = mdx_masked_lines(source) + for start, _, text, _, _ in mdx_js_sources(path="", source=source): + for number in range(start - 1, start - 1 + len(text.splitlines())): + lines[number] = None + prose = "\n".join("" if line is None else line for line in lines) + return [ + (prose[: match.start()].count("\n") + 1, match.group().rstrip()) for match in MDX_JSX_COMMENT.finditer(prose) + ] + + def example_blocks(source: str) -> list[tuple[int, str, str, str]]: blocks: list[tuple[int, str, str, str]] = [] display_directives = { @@ -971,6 +1049,9 @@ def unwrap_static_command(words: list) -> list: r"|(? int: "runuser", "doas", } +RUNNER_COMMANDS: dict[str, int] = { + "ssh": 1, + "watch": 0, + "xargs": 0, + "env": 0, + "sudo": 0, + "nohup": 0, +} +RUNNER_VALUE_OPTIONS: dict[str, set[str]] = { + "ssh": { + "-p", + "-i", + "-o", + "-F", + "-l", + "-L", + "-R", + "-D", + "-J", + "-W", + "-w", + "-m", + "-c", + "-e", + "-Q", + "-S", + "-b", + "-E", + "-O", + }, + "watch": {"-n", "--interval"}, + "xargs": { + "-I", + "-i", + "-n", + "-d", + "-P", + "-s", + "-E", + "-a", + "-L", + "--max-args", + "--max-chars", + "--max-lines", + "--max-procs", + "--delimiter", + "--eof", + "--arg-file", + "--replace", + }, + "env": { + "-u", + "--unset", + "-C", + "--chdir", + "-S", + "--split-string", + "--block-signal", + "--default-signal", + "--ignore-signal", + "--argv0", + }, + "sudo": { + "-u", + "-g", + "-h", + "-p", + "-C", + "-D", + "-r", + "-t", + "-T", + "-U", + "--user", + "--group", + "--host", + "--prompt", + "--close-from", + "--chdir", + "--role", + "--type", + "--command-timeout", + "--other-user", + }, + "nohup": set(), +} + + +def runner_command_index(words: list[str | None], command: str) -> int: + value_options = RUNNER_VALUE_OPTIONS.get(command, set()) + index = 1 + while index < len(words): + word = words[index] + if word is None: + index += 1 + continue + if word == "--": + index += 1 + break + if command == "env" and re.fullmatch(r"[A-Za-z_][A-Za-z0-9_]*=.*", word): + index += 1 + continue + if word in value_options: + index += 2 + continue + if word.startswith("-") and word != "-": + index += 1 + continue + break + return index + + +def runner_trailing_index(static: list[str | None], command: str) -> int: + first = runner_command_index(static, command) + skip = RUNNER_COMMANDS.get(command, 0) + return first + skip if skip and len(static) - first >= 2 else first + + +def runner_trailing_words(words: list, command: str) -> list: + static = [None if word.parts else word.word for word in words] + return words[runner_trailing_index(static, command) :] + + +def nested_runner_entries( + path: str, source: str, offset: int, unit: str, words: list, command: str, symbol: str +) -> list: + if command not in RUNNER_COMMANDS: + return [] + inner = runner_trailing_words(words, command) + if len(inner) != 1 or inner[0].parts or not inner[0].word.strip(): + return [] + try: + bashlex.parse(inner[0].word) + except BASHLEX_FAILURES as exc: + raise ValueError("runner command string requires an adapter") from exc + line = source[: offset + inner[0].pos[0]].count("\n") + 1 + return [(line, path + ".bash", inner[0].word, "bash", f"{symbol}:runner:{command}")] + + +def unwrap_runner_command(words: list, command: str) -> tuple[list, str]: + if command not in SHELL_COMMAND_RUNNERS: + return words, command + first = runner_command_start([None if word.parts else word.word for word in words], command) + for index, word in enumerate(words[first:], start=first): + nested = word.word.rsplit("/", 1)[-1] + if not word.parts and (re.fullmatch(r"python[0-9.]*", nested) or nested in SHELL_NESTED_TARGETS): + return words[index:], nested + return words, command + + +def runner_command_entries( + path: str, + source: str, + offset: int, + unit: str, + runner_words: list, + runner: str, + words: list, + command: str, + symbol: str, +) -> list: + entries = nested_runner_entries(path, source, offset, unit, runner_words, runner, symbol) + if command in RUNNER_COMMANDS and (words is not runner_words or command != runner): + entries.extend(nested_runner_entries(path, source, offset, unit, words, command, symbol)) + return entries + + SHELL_NESTED_TARGETS = {"node", "sh", "bash", "zsh", "eval"} | SQL_CLIENTS | SHELL_UNMODELED COMMENT_MARKERS = re.compile(r"#|//|/\*|--|<#") PLAIN_OPTION = re.compile(r"--?[A-Za-z][A-Za-z0-9_-]*|--") @@ -1196,6 +1444,119 @@ def chunk_can_hold_comment(chunk: str) -> bool: return False +PIPED_PRODUCERS = {"echo", "printf"} +PIPED_STDIN_REDIRECTS = {"<", "<<", "<<-", "<<<", ">", ">>", ">|", "<>"} + + +def scan_source_payload(text: str, language: str) -> tuple[str, str]: + if language not in {"python", "bash", "javascript", "sql"}: + raise ValueError("piped stdin consumer requires an executable-payload adapter") + return text, language + + +def piped_consumer_language(command: str) -> str | None: + if re.fullmatch(r"python[0-9.]*", command): + return "python" + return {"node": "javascript", "sh": "bash", "bash": "bash", "zsh": "bash"}.get(command) + + +def piped_producer_text(words: list) -> str | None: + head = words[0].word.rsplit("/", 1)[-1] + args = words[1:] + if head == "echo": + while args and not args[0].parts and re.fullmatch(r"-[neE]+", args[0].word): + args = args[1:] + else: + while args and not args[0].parts and args[0].word.startswith("-") and args[0].word != "-": + if args[0].word == "--": + args = args[1:] + break + if args[0].word == "-v": + return None + raise ValueError("piped printf option requires an adapter") + if args and not args[0].parts and args[0].word == "--": + args = args[1:] + if any(word.parts for word in args): + raise ValueError("dynamic piped interpreter source requires an adapter") + return " ".join(word.word for word in args) or None + + +def piped_sides(node: bashlex.ast.node) -> list | None: + if node.kind != "pipeline": + return None + commands = [child for child in node.parts if child.kind == "command"] + if len(commands) != 2: + return None + sides = [[part for part in command.parts if part.kind == "word"] for command in commands] + if any(not side or side[0].parts for side in sides): + return None + if any( + part.kind == "redirect" and part.type in PIPED_STDIN_REDIRECTS for command in commands for part in command.parts + ): + return None + if sides[0][0].word.rsplit("/", 1)[-1] not in PIPED_PRODUCERS: + return None + return sides + + +def piped_unmodeled_check(side: list) -> None: + consumer = side[0].word.rsplit("/", 1)[-1] + if consumer not in SHELL_UNMODELED or any(word.parts for word in side[1:]): + return + check_unmodeled_inline(side, consumer) + static = [word.word for word in side] + if "-f" in static or any(word != "-" and not word.startswith("-") for word in static[1:]): + return + if not unmodeled_program_words(side, consumer): + raise ValueError("unmodeled interpreter stdin source requires an adapter") + + +def piped_bash_operands(static: list[str]) -> list[str]: + narrowed: list[str] = [] + skip_operand = False + for word in static: + if skip_operand: + skip_operand = False + elif word in {"-o", "+o"}: + skip_operand = True + else: + narrowed.append(word) + return narrowed + + +def piped_stdin_entries(path: str, source: str, offset: int, unit: str, node: bashlex.ast.node, symbol: str) -> list: + sides = piped_sides(node) + if sides is None: + return [] + producer = sides[0][0].word.rsplit("/", 1)[-1] + language = piped_consumer_language(sides[1][0].word.rsplit("/", 1)[-1]) + if language is None: + piped_unmodeled_check(sides[1]) + return [] + static = [word.word for word in sides[1] if not word.parts] + if len(static) != len(sides[1]): + raise ValueError("dynamic piped interpreter consumer requires an adapter") + if inline_source_index(static, language) is not None: + return [] + operands = piped_bash_operands(static[1:]) if language == "bash" else static[1:] + if any(not word.startswith("-") and word != "-" for word in operands): + return [] + text = piped_producer_text(sides[0]) + if text is None: + return [] + payload, resolved = scan_source_payload(text, language) + line = source[: offset + sides[0][0].pos[0]].count("\n") + 1 + return [(line, path + "." + resolved, payload, resolved, f"{symbol}:pipe:{producer}")] + + +def piped_stdin_payloads(path: str, source: str) -> list[tuple[int, str, str, str, str]]: + try: + trees = bashlex.parse(source) + except BASHLEX_FAILURES: + return [] + return [entry for tree in trees for entry in piped_stdin_entries(path, source, 0, source, tree, "")] + + def shell_command_payloads(path: str, source: str) -> list[tuple[int, str, str, str, str]]: if not SHELL_INLINE_INTERPRETER.search(source): return [] @@ -1227,22 +1588,17 @@ def shell_command_unit(path: str, source: str, offset: int, unit: str, trees: li def visit(node: bashlex.ast.node, symbol: str = "") -> None: if node.kind == "function": symbol = f"{symbol}.{node.name.word}".strip(".") + if node.kind == "pipeline": + result.extend(piped_stdin_entries(path, source, offset, unit, node, symbol)) if node.kind == "command": words = [part for part in node.parts if part.kind == "word"] if words: - command = words[0].word.rsplit("/", 1)[-1] + runner_words = words + runner = words[0].word.rsplit("/", 1)[-1] + command = runner words = unwrap_static_command(words) command = words[0].word.rsplit("/", 1)[-1] - if command in SHELL_COMMAND_RUNNERS: - first = runner_command_start([None if word.parts else word.word for word in words], command) - for index, word in enumerate(words[first:], start=first): - nested = word.word.rsplit("/", 1)[-1] - if not word.parts and ( - re.fullmatch(r"python[0-9.]*", nested) or nested in SHELL_NESTED_TARGETS - ): - words = words[index:] - command = nested - break + words, command = unwrap_runner_command(words, command) if command in SHELL_UNMODELED: check_unmodeled_inline(words, command) inert = command == "uv" and len(words) > 1 and words[1].word not in {"run", "tool", "--"} @@ -1286,6 +1642,9 @@ def visit(node: bashlex.ast.node, symbol: str = "") -> None: text = " ".join(word.word for word in payload_words) line = source[: offset + payload_words[0].pos[0]].count("\n") + 1 result.append((line, path + "." + language, text, language, f"{symbol}:command:{command}")) + result.extend( + runner_command_entries(path, source, offset, unit, runner_words, runner, words, command, symbol) + ) for child in getattr(node, "parts", []): visit(child, symbol) for child in getattr(node, "list", []): @@ -1478,6 +1837,11 @@ def visit(node: yaml.Node | None, symbol: str = "", github_script: bool = False, def nested_sources(path: str, source: str, lang: str) -> list[tuple[int, str, str, str, str]]: + if lang == "mdx": + return [ + (start, path + "." + tag, text, FENCE_LANGUAGES.get(tag, "unsupported"), symbol) + for start, tag, text, symbol in example_blocks(source) + ] + mdx_js_sources(path, source) if lang == "groovy": return groovy_payloads(path, source) if lang == "html+jinja": diff --git a/scripts/comment_syntax.py b/scripts/comment_syntax.py index 1dea2f27df..8b927cef12 100644 --- a/scripts/comment_syntax.py +++ b/scripts/comment_syntax.py @@ -4,18 +4,22 @@ import hashlib import io import json +import os import re import shlex +import subprocess import tokenize from dataclasses import dataclass -from pathlib import PurePosixPath +from pathlib import Path, PurePosixPath import yaml from comment_execution import PythonBindings, python_html_sources from comment_payloads import ( astro_template_comments, bounded_html_template, + mdx_jsx_comments, nested_sources, + piped_stdin_payloads, shell_payloads, sql_template_sources, ) @@ -56,6 +60,7 @@ def identity(self) -> tuple[str, str, str, str, str]: ".htm": "html", ".jinja": "html+jinja", ".j2": "html+jinja", + ".mdx": "mdx", ".js": "javascript", ".mjs": "javascript", ".cjs": "javascript", @@ -164,7 +169,7 @@ def language(path: str) -> str | None: return "make" if name.startswith("Dockerfile"): return "docker" - if path.endswith((".md", ".mdx", ".rst")) and not path.startswith(("_project/", "_blog/")): + if path.endswith((".md", ".rst")) and not path.startswith(("_project/", "_blog/")): return "examples" if not PurePosixPath(path).suffix and path.startswith(OWNED_ROOTS): return "unsupported" @@ -447,6 +452,29 @@ def python_executable_findings(path: str, tree: ast.AST, scopes: list[tuple[int, return result +def resolve_typescript_dir(root: Path | str) -> Path | None: + override = os.environ.get("COMMENT_POLICY_TYPESCRIPT") + if override: + return Path(override) + local = Path(root) / "results-explorer" / "node_modules" / "typescript" + if local.exists(): + return local + try: + raw = subprocess.run( + ["git", "rev-parse", "--git-common-dir"], + cwd=root, + capture_output=True, + text=True, + check=True, + ).stdout.strip() + except (OSError, subprocess.CalledProcessError): + return None + common = Path(raw) + if not common.is_absolute(): + common = Path(root) / common + return (common.parent / "results-explorer" / "node_modules" / "typescript").resolve() + + def javascript_key(path: str, source: str) -> str: suffix = ".tsx" if path.endswith((".tsx", ".jsx")) else ".ts" return hashlib.sha256(source.encode()).hexdigest() + suffix @@ -567,6 +595,13 @@ def mask_embedded_sources(path: str, source: str, lang: str) -> str: for start, _, text, _, _ in shell_payloads(path, source, include_data=True): for index in range(start - 1, start - 1 + len(text.splitlines())): lines[index] = re.sub(r"[^\n]", " ", lines[index]) + try: + piped = piped_stdin_payloads(path, source) + except ValueError: + piped = [] + for start, _, text, _, _ in piped: + for index in range(start - 1, start - 1 + len(text.splitlines())): + lines[index] = re.sub(r"[^\n]", " ", lines[index]) return "".join(lines) + "\n" if lang in {"html", "html+jinja"}: return re.sub( @@ -595,6 +630,12 @@ def astro_findings(path: str, source: str, lang: str) -> list[Finding]: return [Finding(path, line, "comment", text) for line, text in astro_template_comments(source)] +def mdx_findings(path: str, source: str, lang: str) -> list[Finding]: + if lang != "mdx": + return [] + return [Finding(path, line, "comment", text) for line, text in mdx_jsx_comments(source)] + + def scan(path: str, source: str, lang: str, js_results: dict[str, list[dict]] | None = None) -> list[Finding]: try: if source.startswith("#!"): @@ -611,8 +652,8 @@ def scan(path: str, source: str, lang: str, js_results: dict[str, list[dict]] | for f in scan(child_path, text, child_lang, js_results) ] nested.extend(template_coverage(path, source, lang)) - if lang in {"notebook", "examples", "astro"}: - return nested + astro_findings(path, source, lang) + if lang in {"notebook", "examples", "astro", "mdx"}: + return nested + astro_findings(path, source, lang) + mdx_findings(path, source, lang) source = mask_embedded_sources(path, source, lang) if lang == "unsupported": raise ValueError("source language has no registered adapter") diff --git a/scripts/comment_syntax_js.cjs b/scripts/comment_syntax_js.cjs index 1f1e597ef5..b925c06c2f 100644 --- a/scripts/comment_syntax_js.cjs +++ b/scripts/comment_syntax_js.cjs @@ -1,6 +1,19 @@ const fs = require('node:fs'); const path = require('node:path'); -const ts = require(process.env.COMMENT_POLICY_TYPESCRIPT || path.resolve(__dirname, '../results-explorer/node_modules/typescript')); + +function resolveTypescriptPath() { + if (process.env.COMMENT_POLICY_TYPESCRIPT) return process.env.COMMENT_POLICY_TYPESCRIPT; + const local = path.resolve(__dirname, '../results-explorer/node_modules/typescript'); + if (fs.existsSync(local)) return local; + try { + const {execSync} = require('node:child_process'); + const common = execSync('git rev-parse --git-common-dir', {encoding: 'utf8', cwd: __dirname}).trim(); + return path.resolve(__dirname, common, '../results-explorer/node_modules/typescript'); + } catch { + return local; + } +} +const ts = require(resolveTypescriptPath()); const REVIEWED_SQL_WRAPPERS = {queryRows: 0, queryRowsOnce: 0, queryWithTimeout: 1}; diff --git a/tests/unit/scripts/test_comment_policy.py b/tests/unit/scripts/test_comment_policy.py index d59b45a789..c24405c714 100644 --- a/tests/unit/scripts/test_comment_policy.py +++ b/tests/unit/scripts/test_comment_policy.py @@ -1255,6 +1255,38 @@ def test_python_executable_strings_reach_scanner(source: str) -> None: assert [f.text for f in python_findings("a.py", source)] == ["# explanation"] +@pytest.mark.parametrize( + ("source", "text"), + [ + ('import subprocess\nsubprocess.run(["ssh", "host", "python3 -c \'# explanation\'"])', "# explanation"), + ( + 'import subprocess\nsubprocess.run(["watch", "-n", "1", "bash -c \'echo ok # explanation\'"])', + "# explanation", + ), + ('import subprocess\nsubprocess.run(["ssh", "host", "echo ok # bare"])', "# bare"), + ( + 'import subprocess\nsubprocess.run(["xargs", "-I{}", "sh -c \'echo ok # explanation\'"])', + "# explanation", + ), + ('import subprocess\nsubprocess.run(["ssh", "host", "python3", "-c", "# split"])', "# split"), + ], +) +def test_python_runner_command_strings_are_scanned(source: str, text: str) -> None: + assert [f.text for f in python_findings("a.py", source)] == [text] + + +@pytest.mark.parametrize( + "source", + [ + 'import subprocess\nsubprocess.run(["ssh", "host", "ls -l"])', + 'import subprocess\nsubprocess.run(["ssh", host, command])', + 'import subprocess\nsubprocess.run(["xargs", "-I{}", "echo", "hi"])', + ], +) +def test_python_runner_commands_without_static_strings_stay_clean(source: str) -> None: + assert python_findings("a.py", source) == [] + + @pytest.mark.parametrize( "source", [ @@ -1375,6 +1407,97 @@ def test_shell_executable_arguments_are_routed(source: str) -> None: assert [f.text for f in scan("a.sh", source, "bash")] == ["# explanation"] +@pytest.mark.parametrize( + "source", + [ + "echo '# explanation' | python3", + "echo '# explanation' | python", + "echo '# explanation' | python3 -", + "echo -n '# explanation' | sh", + "printf '# explanation' | bash", + "printf '%s\\n' '# explanation' | zsh", + "echo 'echo ok # explanation' | bash", + ], +) +def test_piped_producer_payloads_reach_stdin_interpreters(source: str) -> None: + findings = scan("a.sh", source, "bash") + assert [f.text for f in findings] == ["# explanation"] + assert all("pipe" in f.symbol for f in findings) + + +def test_piped_node_payload_reaches_the_typescript_scanner() -> None: + requests = javascript_requests("a.sh", "echo '// explanation' | node", "bash") + assert list(requests.values()) == ["// explanation"] + + +@pytest.mark.parametrize( + "source", + [ + 'echo "$code" | python3', + "echo hi $name | bash", + "printf '%s\\n' \"$code\" | python3", + "echo x | perl", + "echo x | ruby", + ], +) +def test_piped_dynamic_or_unmodeled_stdin_fails_closed(source: str) -> None: + assert [f.kind for f in scan("a.sh", source, "bash")] == ["coverage-error"] + + +@pytest.mark.parametrize( + "source", + [ + "echo hi | grep x", + "echo '# data' | python3 script.py", + "echo '# data' | python3 -c 'pass'", + "cat file | python3 -", + "echo x | perl script.pl", + "printf -v out '# data' | python3", + ], +) +def test_piped_non_stdin_programs_stay_data(source: str) -> None: + assert scan("a.sh", source, "bash") == [] + + +@pytest.mark.parametrize( + ("source", "text"), + [ + ("ssh host \"python3 -c '# explanation'\"", "# explanation"), + ("ssh -p 22 host \"python3 -c '# explanation'\"", "# explanation"), + ("watch \"bash -c 'echo ok # explanation'\"", "# explanation"), + ("watch -n 1 \"bash -c 'echo ok # explanation'\"", "# explanation"), + ("sudo \"python3 -c '# explanation'\"", "# explanation"), + ("nohup \"python3 -c '# explanation'\"", "# explanation"), + ("xargs -I{} \"sh -c 'echo ok # explanation'\"", "# explanation"), + ("env FOO=bar \"python3 -c '# explanation'\"", "# explanation"), + ('ssh host "echo ok # bare"', "# bare"), + ], +) +def test_runner_command_strings_are_scanned(source: str, text: str) -> None: + findings = scan("a.sh", source, "bash") + assert [f.text for f in findings] == [text] + assert all("runner" in f.symbol for f in findings) + + +def test_split_runner_commands_still_resolve_without_recursion() -> None: + findings = scan("a.sh", "ssh host python3 -c '# split'", "bash") + assert [(f.kind, f.text) for f in findings] == [("comment", "# split")] + assert all("runner" not in f.symbol for f in findings) + + +@pytest.mark.parametrize( + "source", + [ + "ssh host ls -l", + "sudo make install", + 'watch -n 1 "echo ok"', + 'ssh host "ls -l"', + ], +) +def test_runner_commands_without_comments_stay_clean(source: str) -> None: + assert scan("a.sh", source, "bash") == [] + + @pytest.mark.parametrize( "source", [ @@ -2786,10 +2909,10 @@ def test_reviewed_javascript_flows_still_occur() -> None: import shutil import subprocess - from comment_syntax import REVIEWED_JAVASCRIPT_FLOWS + from comment_syntax import REVIEWED_JAVASCRIPT_FLOWS, resolve_typescript_dir - typescript = os.environ.get("COMMENT_POLICY_TYPESCRIPT", str(ROOT / "results-explorer/node_modules/typescript")) - if shutil.which("node") is None or not Path(typescript).exists(): + typescript = resolve_typescript_dir(ROOT) + if shutil.which("node") is None or typescript is None or not typescript.exists(): pytest.skip("the TypeScript package for scripts/comment_syntax_js.cjs is not installed") paths = sorted({path for path, _ in REVIEWED_JAVASCRIPT_FLOWS}) requests = {path: (ROOT / path).read_text(encoding="utf-8") for path in paths} @@ -2804,6 +2927,47 @@ def test_reviewed_javascript_flows_still_occur() -> None: assert set(REVIEWED_JAVASCRIPT_FLOWS) <= observed +def test_typescript_resolution_prefers_environment_then_local(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + from comment_syntax import resolve_typescript_dir + + monkeypatch.setenv("COMMENT_POLICY_TYPESCRIPT", "/env/typescript") + assert resolve_typescript_dir(tmp_path) == Path("/env/typescript") + monkeypatch.delenv("COMMENT_POLICY_TYPESCRIPT") + local = tmp_path / "results-explorer" / "node_modules" / "typescript" + local.mkdir(parents=True) + assert resolve_typescript_dir(tmp_path) == local + + +def test_typescript_resolution_falls_back_to_git_common_dir(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + import comment_syntax + from comment_syntax import resolve_typescript_dir + + monkeypatch.delenv("COMMENT_POLICY_TYPESCRIPT", raising=False) + primary = tmp_path / "primary" / "results-explorer" / "node_modules" / "typescript" + primary.mkdir(parents=True) + + def fake_run(*args: object, **kwargs: object) -> subprocess.CompletedProcess: + return subprocess.CompletedProcess(args, 0, str(tmp_path / "primary" / ".git")) + + monkeypatch.setattr(comment_syntax.subprocess, "run", fake_run) + assert resolve_typescript_dir(tmp_path / "linked-worktree") == primary.resolve() + + +def test_typescript_resolution_returns_none_when_git_is_unavailable( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + import comment_syntax + from comment_syntax import resolve_typescript_dir + + monkeypatch.delenv("COMMENT_POLICY_TYPESCRIPT", raising=False) + + def missing_git(*args: object, **kwargs: object) -> subprocess.CompletedProcess: + raise FileNotFoundError("no git on PATH") + + monkeypatch.setattr(comment_syntax.subprocess, "run", missing_git) + assert resolve_typescript_dir(tmp_path) is None + + @pytest.mark.parametrize( ("source", "kind"), [ @@ -3201,9 +3365,78 @@ def test_unparseable_chunk_with_perl_pod_or_ruby_block_comment_fails_closed() -> assert [f.kind for f in scan("a.sh", ruby, "bash")] == ["coverage-error"] -def test_mdx_pages_are_scanned_like_markdown() -> None: +def test_mdx_pages_scan_prose_fences_and_imports() -> None: from comment_syntax import language - assert language("website/src/content/docs/page.mdx") == "examples" + assert language("website/src/content/docs/page.mdx") == "mdx" source = "import X from './x.astro';\n\n```python\nx = 1 # note\n```\n" - assert [f.text for f in scan("website/src/content/docs/page.mdx", source, "examples", {})] == ["# note"] + rows = {key: [] for key in javascript_requests("website/src/content/docs/page.mdx", source, "mdx")} + assert [f.text for f in scan("website/src/content/docs/page.mdx", source, "mdx", rows)] == ["# note"] + + +@pytest.mark.parametrize( + ("source", "expected"), + [ + ("# Title\n\n{/* explanation */}\n", [(3, "{/* explanation */}")]), + ("{/* first */}\n\n{/* second */}\n", [(1, "{/* first */}"), (3, "{/* second */}")]), + ("
\n {/* explanation */}\n
\n", [(2, "{/* explanation */}")]), + ("{/* multi\nline */}\n", [(1, "{/* multi\nline */}")]), + ], +) +def test_mdx_jsx_comments_are_found(source: str, expected: list[tuple[int, str]]) -> None: + findings = scan("website/src/content/docs/page.mdx", source, "mdx", {}) + assert [(f.kind, f.line, f.text) for f in findings] == [("comment", *item) for item in expected] + + +def test_mdx_prose_and_fences_are_scanned_once() -> None: + source = "```python\n# explanation\n```\n\n{/* prose */}\n" + findings = scan("website/src/content/docs/page.mdx", source, "mdx", {}) + assert [(f.line, f.text) for f in findings] == [(2, "# explanation"), (5, "{/* prose */}")] + + +@pytest.mark.parametrize( + "source", + [ + "import regulations are strict\n", + "export controls are important\n", + "important notes follow\n", + ], +) +def test_mdx_prose_import_lookalikes_are_not_code(source: str) -> None: + assert scan("website/src/content/docs/page.mdx", source, "mdx", {}) == [] + + +@pytest.mark.parametrize( + "source", + [ + "import X from './x'; // explanation\n", + "import {\n A, // explanation\n B,\n} from './x';\n", + "export const meta = 1; // explanation\n", + ], +) +def test_mdx_imports_reach_the_typescript_scanner(source: str) -> None: + requests = javascript_requests("website/src/content/docs/page.mdx", source, "mdx") + assert len(requests) == 1 + rows = { + key: [ + { + "kind": "comment", + "line": 2 if source.startswith("import {\n") else 1, + "text": "// explanation", + "symbol": "", + } + ] + for key in requests + } + findings = scan("website/src/content/docs/page.mdx", source, "mdx", rows) + assert [(f.kind, f.text) for f in findings] == [("comment", "// explanation")] + assert [f.line for f in findings] == [2 if source.startswith("import {\n") else 1] + + +def test_mdx_import_comment_lines_map_to_source_lines() -> None: + source = "# Title\n\nimport X from './x'; // explanation\n" + requests = javascript_requests("website/src/content/docs/page.mdx", source, "mdx") + rows = {key: [{"kind": "comment", "line": 1, "text": "// explanation", "symbol": ""}] for key in requests} + assert [(f.line, f.text) for f in scan("website/src/content/docs/page.mdx", source, "mdx", rows)] == [ + (3, "// explanation") + ] From 2ba792777ef0feb441be4090bb95871d95c45f05 Mon Sep 17 00:00:00 2001 From: Joe Harris Date: Fri, 9 Oct 2026 18:01:29 -0400 Subject: [PATCH 02/17] fix: preserve comment findings in static adapters --- .../audits/remediation-contract-evidence.md | 37 ++++++ docs/internal/comment-policy.md | 16 ++- scripts/comment_execution.py | 12 +- scripts/comment_payloads.py | 117 +++++++++++++++--- scripts/comment_syntax.py | 8 -- tests/unit/scripts/test_comment_policy.py | 58 ++++++--- 6 files changed, 203 insertions(+), 45 deletions(-) diff --git a/_project/audits/remediation-contract-evidence.md b/_project/audits/remediation-contract-evidence.md index 6ffa947b2a..0060c7b46a 100644 --- a/_project/audits/remediation-contract-evidence.md +++ b/_project/audits/remediation-contract-evidence.md @@ -315,3 +315,40 @@ Changed-test Ruff check and format check, codespell 2.4.3, and passed their expected rejection outcome. Docker CLI is unavailable in this environment; no real Compose CLI config or live registry inspection was performed. + +## PR #2821 scanner review findings + +The four Oracle findings were reproduced on the pre-fix branch. Each fix keeps +the original source as the durable input; the scanner adapter output is consumed +by the comment-policy parser and its finding rows. + +- Shell pipeline masking: the shell source producer writes + `echo 'pass' | python3 # explanation`; `mask_embedded_sources` now masks + embedded heredocs only, leaving the shell line for the tokenizer. The + consumer test is + `tests/unit/scripts/test_comment_policy.py::test_piped_interpreter_masking_preserves_trailing_shell_comment`; + it requires the trailing comment and rejects embedded payload leakage. +- Static `echo` and `printf` output: bashlex supplies producer arguments, + `piped_producer_text` reconstructs supported bytes and formats, and the + interpreter scanner consumes the reconstructed source. Positive cases, + including octal escapes, `echo -e`, `%s`, and empty `printf` formats, run in + `test_piped_producer_payloads_reach_stdin_interpreters` and + `test_piped_printf_format_is_evaluated`; `test_piped_node_source_reports_javascript_comment` + exercises the Node consumer. The rejected `%d` conversion and dynamic or + unsupported producers are controls in + `test_piped_dynamic_or_unmodeled_stdin_fails_closed`. +- MDX ESM shape recognition: the original MDX source is retained after + block-comment-tolerant shape validation; `javascript_requests` sends the + complete statement to the TypeScript scanner. Both inter-token forms are + exercised end-to-end by `test_mdx_esm_intertoken_comments_are_reported`; + `test_mdx_prose_import_lookalikes_are_not_code` rejects prose lookalikes. +- Static runner paths with spaces: the shell command AST feeds + `PythonBindings.runner_payload`, which now tests executable basenames rather + than rejecting whitespace anywhere in an absolute path. The positive + `/opt/My Tools/python3` case is in + `test_python_runner_command_strings_are_scanned`; non-source application + arguments remain covered by `test_shell_application_arguments_are_not_executable_source`. + +The focused scanner regressions passed 36 tests; the full policy unit file +passed 706 tests with `-n 0`. `make comment-policy-check` scanned 5,749 +files with zero violations and passed all 19 native syntax tests. diff --git a/docs/internal/comment-policy.md b/docs/internal/comment-policy.md index 789dd51ebb..311cb44b7f 100644 --- a/docs/internal/comment-policy.md +++ b/docs/internal/comment-policy.md @@ -144,6 +144,9 @@ Python is read with its AST and tokenizer, including standalone strings and runtime docstring assignments; JavaScript and TypeScript with the isolated TypeScript parser; SQL-valued strings and execution-sink arguments as SQL; other formats with Pygments lexers; embedded code with its own language check. +Shell heredocs and literal `echo`/`printf` pipelines into supported stdin +interpreters are scanned; MDX top-level ESM and JSX block comments are +extracted. Unknown input is a coverage error, never a pass, and under blocking enforcement or in strict mode changed files and completed scopes always reject it. While enforcement is advisory a coverage error is reported and counted but does not @@ -172,8 +175,9 @@ Known gaps, each a place where a comment can pass unreported: - Unknown programs that take `-c` or `-e` as data are listed in `DATA_FLAG_PROGRAMS` in `scripts/comment_execution.py`; any other program given such a flag is reported. -- Commands passed to a runner such as `ssh` or `watch` as a dynamic string - are not split, so an interpreter inside that string is not followed. +- Static inner commands passed as a single argument to supported runners such as + `ssh`, `watch` or `xargs` are followed; dynamically assembled commands are + not. - HTML comments and MyST `%` lines in Markdown prose are not scanned. In the maintained docs they are copyright headers, generator start and end markers, `` markers read by `scripts/blog_content_validation.py`, @@ -185,8 +189,10 @@ Known gaps, each a place where a comment can pass unreported: tuning note and the AWS Glue job script. - A program piped to an interpreter on stdin from a producer other than `echo` or `printf` (such as `curl ... | sh`) is not scanned. -- A whole-file pinned notice in `quality/comment-cleanup-scope.json` (such as - `_sources/tpc-h/PATCHES.md`) is verified against its recorded digest, but - changes to the file are not guarded to require an accompanying pin update on branches. +- Whole-file pinned notices in `quality/comment-cleanup-scope.json` are checked + against their recorded digest, but changed files do not yet require a pin + update on the same branch. +- Unsupported `printf` formats or `echo -e` escapes in a recognized pipeline + produce a coverage error. - JSX `{// ...}` line-comment expressions in MDX prose are not extracted, so comments in that form outside Markdown code fences are not inspected. diff --git a/scripts/comment_execution.py b/scripts/comment_execution.py index 821dc22c90..78476a9e50 100644 --- a/scripts/comment_execution.py +++ b/scripts/comment_execution.py @@ -652,10 +652,14 @@ def runner_payload( except ValueError: return args[0], "unsupported", None for index, word in enumerate(words[first:], start=first): - nested = word.rsplit("/", 1)[-1] if word and not any(char.isspace() for char in word) else None - if nested and ( - nested.startswith("python") - or nested in {"node", "sh", "bash", "zsh"} | SQL_CLIENTS | UNMODELED_INTERPRETERS + nested = word.rsplit("/", 1)[-1] if word else None + if ( + nested + and not any(char.isspace() for char in nested) + and ( + nested.startswith("python") + or nested in {"node", "sh", "bash", "zsh"} | SQL_CLIENTS | UNMODELED_INTERPRETERS + ) ): return self.process_payload(args[index:]) if name not in DATA_FLAG_PROGRAMS and any( diff --git a/scripts/comment_payloads.py b/scripts/comment_payloads.py index 333bb85fa0..c63a958474 100644 --- a/scripts/comment_payloads.py +++ b/scripts/comment_payloads.py @@ -375,10 +375,11 @@ def mdx_js_sources(path: str, source: str) -> list[tuple[int, str, str, str, str if line is not None and MDX_STATEMENT_START.match(line): end = mdx_statement_end(lines, index) text = "\n".join(str(lines[number]) for number in range(index, end)) + shape = re.sub(r"/\*.*?\*/", " ", text, flags=re.S) keep = ( - MDX_IMPORT_FROM.search(text) or MDX_IMPORT_BARE.match(text) + MDX_IMPORT_FROM.search(shape) or MDX_IMPORT_BARE.match(shape) if line.startswith("import") - else MDX_EXPORT_SHAPE.match(text) + else MDX_EXPORT_SHAPE.match(shape) ) if keep: result.append((index + 1, path + ".tsx", text, "javascript", f"mdx:{len(result)}")) @@ -1460,25 +1461,113 @@ def piped_consumer_language(command: str) -> str | None: return {"node": "javascript", "sh": "bash", "bash": "bash", "zsh": "bash"}.get(command) +def shell_output_escapes(text: str) -> tuple[str, bool]: + result = [] + index = 0 + escaped = { + "a": "\a", + "b": "\b", + "f": "\f", + "n": "\n", + "r": "\r", + "t": "\t", + "v": "\v", + "\\": "\\", + } + while index < len(text): + if text[index] != "\\": + result.append(text[index]) + index += 1 + continue + index += 1 + if index == len(text): + raise ValueError("trailing escape in piped producer output requires an adapter") + char = text[index] + if char == "c": + return "".join(result), True + if char in escaped: + result.append(escaped[char]) + index += 1 + continue + if char in "01234567": + end = index + 1 + while end < min(index + 3, len(text)) and text[end] in "01234567": + end += 1 + result.append(chr(int(text[index:end], 8))) + index = end + continue + if char == "x" and index + 1 < len(text) and text[index + 1] in "0123456789abcdefABCDEF": + end = index + 2 + if end < len(text) and text[end] in "0123456789abcdefABCDEF": + end += 1 + result.append(chr(int(text[index + 1 : end], 16))) + index = end + continue + raise ValueError("unsupported escape in piped producer output requires an adapter") + return "".join(result), False + + +def piped_printf_text(args: list) -> str: + if any(word.parts for word in args): + raise ValueError("dynamic piped interpreter source requires an adapter") + if not args: + return "" + format_text, stopped = shell_output_escapes(args[0].word) + if stopped: + return format_text + values = [word.word for word in args[1:]] + result = [] + value_index = 0 + while True: + index = 0 + while index < len(format_text): + char = format_text[index] + if char != "%": + result.append(char) + index += 1 + elif format_text[index : index + 2] == "%%": + result.append("%") + index += 2 + elif format_text[index : index + 2] == "%s": + result.append(values[value_index] if value_index < len(values) else "") + value_index += 1 + index += 2 + else: + raise ValueError("unsupported piped printf format requires an adapter") + if "%s" not in format_text.replace("%%", "") or value_index >= len(values): + break + return "".join(result) + + def piped_producer_text(words: list) -> str | None: head = words[0].word.rsplit("/", 1)[-1] args = words[1:] if head == "echo": + no_newline = False + escapes = False while args and not args[0].parts and re.fullmatch(r"-[neE]+", args[0].word): + for option in args[0].word[1:]: + no_newline |= option == "n" + escapes = option == "e" if option in "eE" else escapes args = args[1:] - else: - while args and not args[0].parts and args[0].word.startswith("-") and args[0].word != "-": - if args[0].word == "--": - args = args[1:] - break - if args[0].word == "-v": - return None - raise ValueError("piped printf option requires an adapter") - if args and not args[0].parts and args[0].word == "--": + if any(word.parts for word in args): + raise ValueError("dynamic piped interpreter source requires an adapter") + text = " ".join(word.word for word in args) + if escapes: + text, stopped = shell_output_escapes(text) + if stopped: + return text or None + return (text + ("" if no_newline else "\n")) or None + while args and not args[0].parts and args[0].word.startswith("-") and args[0].word != "-": + if args[0].word == "--": args = args[1:] - if any(word.parts for word in args): - raise ValueError("dynamic piped interpreter source requires an adapter") - return " ".join(word.word for word in args) or None + break + if args[0].word == "-v": + return None + raise ValueError("piped printf option requires an adapter") + if args and not args[0].parts and args[0].word == "--": + args = args[1:] + return piped_printf_text(args) or None def piped_sides(node: bashlex.ast.node) -> list | None: diff --git a/scripts/comment_syntax.py b/scripts/comment_syntax.py index 8b927cef12..c9426b77f2 100644 --- a/scripts/comment_syntax.py +++ b/scripts/comment_syntax.py @@ -19,7 +19,6 @@ bounded_html_template, mdx_jsx_comments, nested_sources, - piped_stdin_payloads, shell_payloads, sql_template_sources, ) @@ -595,13 +594,6 @@ def mask_embedded_sources(path: str, source: str, lang: str) -> str: for start, _, text, _, _ in shell_payloads(path, source, include_data=True): for index in range(start - 1, start - 1 + len(text.splitlines())): lines[index] = re.sub(r"[^\n]", " ", lines[index]) - try: - piped = piped_stdin_payloads(path, source) - except ValueError: - piped = [] - for start, _, text, _, _ in piped: - for index in range(start - 1, start - 1 + len(text.splitlines())): - lines[index] = re.sub(r"[^\n]", " ", lines[index]) return "".join(lines) + "\n" if lang in {"html", "html+jinja"}: return re.sub( diff --git a/tests/unit/scripts/test_comment_policy.py b/tests/unit/scripts/test_comment_policy.py index c24405c714..552f6872ec 100644 --- a/tests/unit/scripts/test_comment_policy.py +++ b/tests/unit/scripts/test_comment_policy.py @@ -1269,6 +1269,10 @@ def test_python_executable_strings_reach_scanner(source: str) -> None: "# explanation", ), ('import subprocess\nsubprocess.run(["ssh", "host", "python3", "-c", "# split"])', "# split"), + ( + 'import subprocess\nsubprocess.run(["docker", "exec", "container", "/opt/My Tools/python3", "-c", "# hidden"])', + "# hidden", + ), ], ) def test_python_runner_command_strings_are_scanned(source: str, text: str) -> None: @@ -1408,26 +1412,36 @@ def test_shell_executable_arguments_are_routed(source: str) -> None: @pytest.mark.parametrize( - "source", + ("source", "expected"), [ - "echo '# explanation' | python3", - "echo '# explanation' | python", - "echo '# explanation' | python3 -", - "echo -n '# explanation' | sh", - "printf '# explanation' | bash", - "printf '%s\\n' '# explanation' | zsh", - "echo 'echo ok # explanation' | bash", + ("echo '# explanation' | python3", "# explanation"), + ("echo '# explanation' | python", "# explanation"), + ("echo '# explanation' | python3 -", "# explanation"), + ("echo -n '# explanation' | sh", "# explanation"), + ("printf '# explanation' | bash", "# explanation"), + ("printf '%s\\n' '# explanation' | zsh", "# explanation"), + ("printf '\\043 hidden\\n' | sh", "# hidden"), + ("echo -e '\\043 hidden' | sh", "# hidden"), ], ) -def test_piped_producer_payloads_reach_stdin_interpreters(source: str) -> None: +def test_piped_producer_payloads_reach_stdin_interpreters(source: str, expected: str) -> None: findings = scan("a.sh", source, "bash") - assert [f.text for f in findings] == ["# explanation"] - assert all("pipe" in f.symbol for f in findings) + assert [finding.text for finding in findings] == [expected] + assert all("pipe" in finding.symbol for finding in findings) + + +def test_piped_interpreter_masking_preserves_trailing_shell_comment() -> None: + findings = scan("a.sh", "echo 'pass' | python3 # explanation", "bash") + assert [(finding.kind, finding.text) for finding in findings] == [("comment", "# explanation")] + +def test_piped_printf_format_is_evaluated() -> None: + assert scan("a.sh", "printf '%s\\n' 'pass' | python3", "bash") == [] -def test_piped_node_payload_reaches_the_typescript_scanner() -> None: - requests = javascript_requests("a.sh", "echo '// explanation' | node", "bash") - assert list(requests.values()) == ["// explanation"] + +def test_piped_node_source_reports_javascript_comment() -> None: + findings = scan_sources(ROOT, {"a.sh": b"echo '// explanation' | node"}, policy()) + assert [(finding.kind, finding.text) for finding in findings] == [("comment", "// explanation")] @pytest.mark.parametrize( @@ -1436,7 +1450,10 @@ def test_piped_node_payload_reaches_the_typescript_scanner() -> None: 'echo "$code" | python3', "echo hi $name | bash", "printf '%s\\n' \"$code\" | python3", + "printf '%d\\n' 1 | python3", "echo x | perl", + r"echo -e '\e hidden' | bash", + r"echo -e '\E hidden' | bash", "echo x | ruby", ], ) @@ -3433,6 +3450,19 @@ def test_mdx_imports_reach_the_typescript_scanner(source: str) -> None: assert [f.line for f in findings] == [2 if source.startswith("import {\n") else 1] +@pytest.mark.parametrize( + ("source", "comment"), + [ + ("export /* hidden */ const value = 1;", "/* hidden */"), + ("import /* hidden */ './x.js';", "/* hidden */"), + ], +) +def test_mdx_esm_intertoken_comments_are_reported(source: str, comment: str) -> None: + path = "website/src/content/docs/page.mdx" + findings = scan_sources(ROOT, {path: source.encode()}, policy()) + assert [(finding.kind, finding.text) for finding in findings] == [("comment", comment)] + + def test_mdx_import_comment_lines_map_to_source_lines() -> None: source = "# Title\n\nimport X from './x'; // explanation\n" requests = javascript_requests("website/src/content/docs/page.mdx", source, "mdx") From 221532a40cbde6ee0b96a497303b6e15349d4c55 Mon Sep 17 00:00:00 2001 From: Joe Harris Date: Fri, 9 Oct 2026 20:59:23 -0400 Subject: [PATCH 03/17] fix: close five additional scanner review findings --- .../audits/remediation-contract-evidence.md | 33 +++++++++-- docs/internal/comment-policy.md | 2 + scripts/comment_execution.py | 8 +-- scripts/comment_payloads.py | 57 +++++++++++++++---- tests/unit/scripts/test_comment_policy.py | 27 +++++++++ 5 files changed, 106 insertions(+), 21 deletions(-) diff --git a/_project/audits/remediation-contract-evidence.md b/_project/audits/remediation-contract-evidence.md index 0060c7b46a..9165203ff6 100644 --- a/_project/audits/remediation-contract-evidence.md +++ b/_project/audits/remediation-contract-evidence.md @@ -318,7 +318,7 @@ performed. ## PR #2821 scanner review findings -The four Oracle findings were reproduced on the pre-fix branch. Each fix keeps +The nine Oracle findings were reproduced on the pre-fix branch. Each fix keeps the original source as the durable input; the scanner adapter output is consumed by the comment-policy parser and its finding rows. @@ -348,7 +348,30 @@ by the comment-policy parser and its finding rows. `/opt/My Tools/python3` case is in `test_python_runner_command_strings_are_scanned`; non-source application arguments remain covered by `test_shell_application_arguments_are_not_executable_source`. - -The focused scanner regressions passed 36 tests; the full policy unit file -passed 706 tests with `-n 0`. `make comment-policy-check` scanned 5,749 -files with zero violations and passed all 19 native syntax tests. +- Absolute interpreter paths in pipelines: `SHELL_INLINE_INTERPRETER` now + admits recognized executable basenames behind a path prefix, so + `echo '# hidden' | /usr/bin/python3` reaches the consumer scanner. The + regression is in `test_piped_producer_payloads_reach_stdin_interpreters`. +- Pipeline redirects: descriptor analysis preserves consumer stdout and + stderr plus producer stderr, while requiring coverage for a consumer stdin + override. The positive and fail-closed controls are + `test_piped_unrelated_redirections_preserve_stdin` and + `test_piped_consumer_stdin_redirection_fails_closed`. +- Bash `echo -e` octal escapes: the decoder treats `\0` followed by up to three + octal digits as one escape, separately from the `printf` format path. + `test_piped_producer_payloads_reach_stdin_interpreters` exercises + `\0043 hidden` as `# hidden`. +- Runner command string ordering: Python runner payloads now scan a complete + static shell command string before matching an interpreter basename at its + suffix. `test_python_runner_command_strings_are_scanned` covers a comment + followed by `/usr/bin/python3`. +- Multiline MDX block comments: statement extraction tracks block-comment + boundaries before shape validation and passes the complete statement to the + TypeScript scanner. The multiline case in + `test_mdx_esm_intertoken_comments_are_reported` requires the comment. + + +The original focused scanner regressions passed 36 tests; the five additional +fix groups passed 25 tests. The full policy unit file passed 716 tests with +`-n 0`. `make comment-policy-check` scanned 5,749 files with zero violations +and passed all 19 native syntax tests. diff --git a/docs/internal/comment-policy.md b/docs/internal/comment-policy.md index 311cb44b7f..8667fee06d 100644 --- a/docs/internal/comment-policy.md +++ b/docs/internal/comment-policy.md @@ -147,6 +147,8 @@ formats with Pygments lexers; embedded code with its own language check. Shell heredocs and literal `echo`/`printf` pipelines into supported stdin interpreters are scanned; MDX top-level ESM and JSX block comments are extracted. +Executable paths are matched by basename; consumer output redirects preserve +piped input, while redirected stdin fails closed. Unknown input is a coverage error, never a pass, and under blocking enforcement or in strict mode changed files and completed scopes always reject it. While enforcement is advisory a coverage error is reported and counted but does not diff --git a/scripts/comment_execution.py b/scripts/comment_execution.py index 78476a9e50..54a619684f 100644 --- a/scripts/comment_execution.py +++ b/scripts/comment_execution.py @@ -647,6 +647,10 @@ def process_payload(self, args: list[ast.expr]) -> tuple[ast.AST, str, str | Non def runner_payload( self, args: list[ast.expr], words: list[str | None], name: str ) -> tuple[ast.AST, str, str | None] | None: + if name in RUNNER_COMMANDS: + inner = self.runner_string_args(args, words, name) + if inner is not None: + return inner try: first = runner_command_start(words, name) except ValueError: @@ -666,10 +670,6 @@ def runner_payload( word is not None and INLINE_SHAPED_FLAG.fullmatch(word) for word in words[1:] ): return args[0], "unsupported", None - if name in RUNNER_COMMANDS: - inner = self.runner_string_args(args, words, name) - if inner is not None: - return inner return None def runner_string_args( diff --git a/scripts/comment_payloads.py b/scripts/comment_payloads.py index c63a958474..9abe3c8878 100644 --- a/scripts/comment_payloads.py +++ b/scripts/comment_payloads.py @@ -341,6 +341,7 @@ def mdx_masked_lines(source: str) -> list[str | None]: def mdx_statement_end(lines: list[str | None], start: int) -> int: depth = 0 index = start + in_block_comment = False while index < len(lines): line = lines[index] if line is None: @@ -348,6 +349,14 @@ def mdx_statement_end(lines: list[str | None], start: int) -> int: cursor = 0 while cursor < len(line): char = line[cursor] + if in_block_comment: + closing = line.find("*/", cursor) + if closing < 0: + cursor = len(line) + continue + cursor = closing + 2 + in_block_comment = False + continue if char in "'\"`": closing = line.find(char, cursor + 1) while closing >= 0 and line[closing - 1] == "\\": @@ -356,12 +365,18 @@ def mdx_statement_end(lines: list[str | None], start: int) -> int: raise ValueError("unterminated string in MDX JavaScript statement") cursor = closing + 1 continue + if line.startswith("//", cursor): + break + if line.startswith("/*", cursor): + in_block_comment = True + cursor += 2 + continue depth += (char in "{([") - (char in "})]") cursor += 1 index += 1 - if depth <= 0 and (index - start > 1 or not MDX_STATEMENT_CONTINUE.search(line)): + if depth <= 0 and not in_block_comment and (index - start > 1 or not MDX_STATEMENT_CONTINUE.search(line)): break - if depth > 0: + if depth > 0 or in_block_comment: raise ValueError("unterminated JavaScript statement in MDX source") return index @@ -1050,8 +1065,8 @@ def unwrap_static_command(words: list) -> list: r"|(? bool: PIPED_PRODUCERS = {"echo", "printf"} -PIPED_STDIN_REDIRECTS = {"<", "<<", "<<-", "<<<", ">", ">>", ">|", "<>"} def scan_source_payload(text: str, language: str) -> tuple[str, str]: @@ -1461,7 +1475,7 @@ def piped_consumer_language(command: str) -> str | None: return {"node": "javascript", "sh": "bash", "bash": "bash", "zsh": "bash"}.get(command) -def shell_output_escapes(text: str) -> tuple[str, bool]: +def shell_output_escapes(text: str, *, zero_prefix_octal: bool = False) -> tuple[str, bool]: result = [] index = 0 escaped = { @@ -1490,8 +1504,9 @@ def shell_output_escapes(text: str) -> tuple[str, bool]: index += 1 continue if char in "01234567": + limit = index + (4 if zero_prefix_octal and char == "0" else 3) end = index + 1 - while end < min(index + 3, len(text)) and text[end] in "01234567": + while end < min(limit, len(text)) and text[end] in "01234567": end += 1 result.append(chr(int(text[index:end], 8))) index = end @@ -1554,7 +1569,7 @@ def piped_producer_text(words: list) -> str | None: raise ValueError("dynamic piped interpreter source requires an adapter") text = " ".join(word.word for word in args) if escapes: - text, stopped = shell_output_escapes(text) + text, stopped = shell_output_escapes(text, zero_prefix_octal=True) if stopped: return text or None return (text + ("" if no_newline else "\n")) or None @@ -1570,6 +1585,23 @@ def piped_producer_text(words: list) -> str | None: return piped_printf_text(args) or None +def piped_redirect_effect(node: bashlex.ast.node) -> tuple[bool, bool]: + commands = [child for child in node.parts if child.kind == "command"] + stdin_redirects = {"<", "<<", "<<-", "<<<", "<>", "<&"} + producer_stdout_redirected = False + consumer_stdin_redirected = False + for command_index, command in enumerate(commands): + for part in command.parts: + if part.kind != "redirect": + continue + descriptor = part.input + if descriptor is None: + descriptor = 0 if part.type in stdin_redirects else 1 + producer_stdout_redirected |= command_index == 0 and descriptor == 1 + consumer_stdin_redirected |= command_index == 1 and descriptor == 0 + return producer_stdout_redirected, consumer_stdin_redirected + + def piped_sides(node: bashlex.ast.node) -> list | None: if node.kind != "pipeline": return None @@ -1579,10 +1611,6 @@ def piped_sides(node: bashlex.ast.node) -> list | None: sides = [[part for part in command.parts if part.kind == "word"] for command in commands] if any(not side or side[0].parts for side in sides): return None - if any( - part.kind == "redirect" and part.type in PIPED_STDIN_REDIRECTS for command in commands for part in command.parts - ): - return None if sides[0][0].word.rsplit("/", 1)[-1] not in PIPED_PRODUCERS: return None return sides @@ -1617,6 +1645,11 @@ def piped_stdin_entries(path: str, source: str, offset: int, unit: str, node: ba sides = piped_sides(node) if sides is None: return [] + producer_stdout_redirected, consumer_stdin_redirected = piped_redirect_effect(node) + if producer_stdout_redirected: + return [] + if consumer_stdin_redirected: + raise ValueError("piped interpreter stdin redirection requires an adapter") producer = sides[0][0].word.rsplit("/", 1)[-1] language = piped_consumer_language(sides[1][0].word.rsplit("/", 1)[-1]) if language is None: diff --git a/tests/unit/scripts/test_comment_policy.py b/tests/unit/scripts/test_comment_policy.py index 552f6872ec..fc8c113c61 100644 --- a/tests/unit/scripts/test_comment_policy.py +++ b/tests/unit/scripts/test_comment_policy.py @@ -1273,6 +1273,10 @@ def test_python_executable_strings_reach_scanner(source: str) -> None: 'import subprocess\nsubprocess.run(["docker", "exec", "container", "/opt/My Tools/python3", "-c", "# hidden"])', "# hidden", ), + ( + 'import subprocess\nsubprocess.run(["ssh", "host", "true # hidden\\n/usr/bin/python3"])', + "# hidden", + ), ], ) def test_python_runner_command_strings_are_scanned(source: str, text: str) -> None: @@ -1422,6 +1426,9 @@ def test_shell_executable_arguments_are_routed(source: str) -> None: ("printf '%s\\n' '# explanation' | zsh", "# explanation"), ("printf '\\043 hidden\\n' | sh", "# hidden"), ("echo -e '\\043 hidden' | sh", "# hidden"), + ("echo -e '\\0043 hidden' | bash", "# hidden"), + ("echo '# hidden' | /usr/bin/python3", "# hidden"), + ('echo "# hidden" | "/opt/My Tools/python3"', "# hidden"), ], ) def test_piped_producer_payloads_reach_stdin_interpreters(source: str, expected: str) -> None: @@ -1439,6 +1446,24 @@ def test_piped_printf_format_is_evaluated() -> None: assert scan("a.sh", "printf '%s\\n' 'pass' | python3", "bash") == [] +@pytest.mark.parametrize( + "source", + [ + "echo '# hidden' | python3 > /dev/null", + "echo '# hidden' | python3 2>/dev/null", + "echo '# hidden' 2>/dev/null | python3", + ], +) +def test_piped_unrelated_redirections_preserve_stdin(source: str) -> None: + findings = scan("a.sh", source, "bash") + assert [(finding.kind, finding.text) for finding in findings] == [("comment", "# hidden")] + + +def test_piped_consumer_stdin_redirection_fails_closed() -> None: + findings = scan("a.sh", "echo '# hidden' | python3 < /dev/null", "bash") + assert [finding.kind for finding in findings] == ["coverage-error"] + + def test_piped_node_source_reports_javascript_comment() -> None: findings = scan_sources(ROOT, {"a.sh": b"echo '// explanation' | node"}, policy()) assert [(finding.kind, finding.text) for finding in findings] == [("comment", "// explanation")] @@ -1488,6 +1513,7 @@ def test_piped_non_stdin_programs_stay_data(source: str) -> None: ("xargs -I{} \"sh -c 'echo ok # explanation'\"", "# explanation"), ("env FOO=bar \"python3 -c '# explanation'\"", "# explanation"), ('ssh host "echo ok # bare"', "# bare"), + ('ssh host "true # hidden\n/usr/bin/python3"', "# hidden"), ], ) def test_runner_command_strings_are_scanned(source: str, text: str) -> None: @@ -3455,6 +3481,7 @@ def test_mdx_imports_reach_the_typescript_scanner(source: str) -> None: [ ("export /* hidden */ const value = 1;", "/* hidden */"), ("import /* hidden */ './x.js';", "/* hidden */"), + ("import /*\n hidden\n*/ './x.js';", "/*\n hidden\n*/"), ], ) def test_mdx_esm_intertoken_comments_are_reported(source: str, comment: str) -> None: From 49bb2b10ff4d33c521d6063c430d9bab14acc29d Mon Sep 17 00:00:00 2001 From: Joe Harris Date: Sat, 10 Oct 2026 09:03:25 -0400 Subject: [PATCH 04/17] fix(scanner): correct octal bytes, printf \c expansion, MDX multiline imports, and pipeline routing --- scripts/comment_payloads.py | 88 ++++++++++++++--------- tests/unit/scripts/test_comment_policy.py | 6 ++ 2 files changed, 61 insertions(+), 33 deletions(-) diff --git a/scripts/comment_payloads.py b/scripts/comment_payloads.py index 9abe3c8878..1b764f1af6 100644 --- a/scripts/comment_payloads.py +++ b/scripts/comment_payloads.py @@ -326,7 +326,7 @@ def astro_expression_sources(path: str, source: str) -> list[tuple[int, str, str MDX_IMPORT_FROM = re.compile(r"\bfrom\s+['\"][^'\"\n]+['\"]") MDX_IMPORT_BARE = re.compile(r"^import\s+['\"]") MDX_EXPORT_SHAPE = re.compile(r"^export\s+(?:default|const|let|var|function|class|async|\{|\*)") -MDX_STATEMENT_CONTINUE = re.compile(r"(?:[{(\[=,:+*/?&|>-]|\bfrom|\bdefault)\s*$") +MDX_STATEMENT_CONTINUE = re.compile(r"(?:[{(\[=,:+*/?&|>-]|\bfrom|\bdefault|\bimport|\bexport)\s*$") def mdx_masked_lines(source: str) -> list[str | None]: @@ -374,8 +374,14 @@ def mdx_statement_end(lines: list[str | None], start: int) -> int: depth += (char in "{([") - (char in "})]") cursor += 1 index += 1 - if depth <= 0 and not in_block_comment and (index - start > 1 or not MDX_STATEMENT_CONTINUE.search(line)): - break + is_import = lines[start].startswith("import") + semicolon = bool(re.search(r";\s*(?://.*)?$", line)) + if depth <= 0 and not in_block_comment: + if is_import: + if semicolon or MDX_IMPORT_FROM.search(line) or MDX_IMPORT_BARE.match(line): + break + elif semicolon or not MDX_STATEMENT_CONTINUE.search(line): + break if depth > 0 or in_block_comment: raise ValueError("unterminated JavaScript statement in MDX source") return index @@ -1508,7 +1514,7 @@ def shell_output_escapes(text: str, *, zero_prefix_octal: bool = False) -> tuple end = index + 1 while end < min(limit, len(text)) and text[end] in "01234567": end += 1 - result.append(chr(int(text[index:end], 8))) + result.append(chr(int(text[index:end], 8) & 0xFF)) index = end continue if char == "x" and index + 1 < len(text) and text[index + 1] in "0123456789abcdefABCDEF": @@ -1527,31 +1533,31 @@ def piped_printf_text(args: list) -> str: raise ValueError("dynamic piped interpreter source requires an adapter") if not args: return "" - format_text, stopped = shell_output_escapes(args[0].word) - if stopped: - return format_text + format_raw = args[0].word values = [word.word for word in args[1:]] - result = [] + expanded = [] value_index = 0 while True: index = 0 - while index < len(format_text): - char = format_text[index] + while index < len(format_raw): + char = format_raw[index] if char != "%": - result.append(char) + expanded.append(char) index += 1 - elif format_text[index : index + 2] == "%%": - result.append("%") + elif format_raw[index : index + 2] == "%%": + expanded.append("%%") index += 2 - elif format_text[index : index + 2] == "%s": - result.append(values[value_index] if value_index < len(values) else "") + elif format_raw[index : index + 2] == "%s": + val = values[value_index] if value_index < len(values) else "" + expanded.append(val.replace("\\", "\\\\")) value_index += 1 index += 2 else: raise ValueError("unsupported piped printf format requires an adapter") - if "%s" not in format_text.replace("%%", "") or value_index >= len(values): + if "%s" not in format_raw.replace("%%", "") or value_index >= len(values): break - return "".join(result) + format_text, _ = shell_output_escapes("".join(expanded)) + return format_text def piped_producer_text(words: list) -> str | None: @@ -1585,8 +1591,7 @@ def piped_producer_text(words: list) -> str | None: return piped_printf_text(args) or None -def piped_redirect_effect(node: bashlex.ast.node) -> tuple[bool, bool]: - commands = [child for child in node.parts if child.kind == "command"] +def piped_redirect_effect(commands: list) -> tuple[bool, bool]: stdin_redirects = {"<", "<<", "<<-", "<<<", "<>", "<&"} producer_stdout_redirected = False consumer_stdin_redirected = False @@ -1597,8 +1602,12 @@ def piped_redirect_effect(node: bashlex.ast.node) -> tuple[bool, bool]: descriptor = part.input if descriptor is None: descriptor = 0 if part.type in stdin_redirects else 1 - producer_stdout_redirected |= command_index == 0 and descriptor == 1 - consumer_stdin_redirected |= command_index == 1 and descriptor == 0 + if command_index == 0 and descriptor == 1: + output_target = getattr(part, "output", None) + if not (part.type == ">&" and output_target == 1): + producer_stdout_redirected = True + if command_index == 1 and descriptor == 0: + consumer_stdin_redirected = True return producer_stdout_redirected, consumer_stdin_redirected @@ -1606,14 +1615,26 @@ def piped_sides(node: bashlex.ast.node) -> list | None: if node.kind != "pipeline": return None commands = [child for child in node.parts if child.kind == "command"] - if len(commands) != 2: + if len(commands) < 2: return None sides = [[part for part in command.parts if part.kind == "word"] for command in commands] if any(not side or side[0].parts for side in sides): return None if sides[0][0].word.rsplit("/", 1)[-1] not in PIPED_PRODUCERS: return None - return sides + unwrapped_consumer = sides[1] + if not any(w.parts for w in sides[1]): + head_cmd = sides[1][0].word.rsplit("/", 1)[-1] + if head_cmd == "env" and len(sides[1]) > 1: + idx = 1 + while idx < len(sides[1]) and ( + re.match(r"[A-Za-z_][A-Za-z0-9_]*=", sides[1][idx].word) + or sides[1][idx].word in {"-i", "--ignore-environment"} + ): + idx += 1 + if idx < len(sides[1]) and not sides[1][idx].word.startswith("-"): + unwrapped_consumer = sides[1][idx:] + return [sides[0], unwrapped_consumer, commands[:2]] def piped_unmodeled_check(side: list) -> None: @@ -1642,32 +1663,33 @@ def piped_bash_operands(static: list[str]) -> list[str]: def piped_stdin_entries(path: str, source: str, offset: int, unit: str, node: bashlex.ast.node, symbol: str) -> list: - sides = piped_sides(node) - if sides is None: + pipe_info = piped_sides(node) + if pipe_info is None: return [] - producer_stdout_redirected, consumer_stdin_redirected = piped_redirect_effect(node) + producer_words, consumer_words, pair_commands = pipe_info + producer_stdout_redirected, consumer_stdin_redirected = piped_redirect_effect(pair_commands) if producer_stdout_redirected: return [] if consumer_stdin_redirected: raise ValueError("piped interpreter stdin redirection requires an adapter") - producer = sides[0][0].word.rsplit("/", 1)[-1] - language = piped_consumer_language(sides[1][0].word.rsplit("/", 1)[-1]) + producer = producer_words[0].word.rsplit("/", 1)[-1] + language = piped_consumer_language(consumer_words[0].word.rsplit("/", 1)[-1]) if language is None: - piped_unmodeled_check(sides[1]) + piped_unmodeled_check(consumer_words) return [] - static = [word.word for word in sides[1] if not word.parts] - if len(static) != len(sides[1]): + static = [word.word for word in consumer_words if not word.parts] + if len(static) != len(consumer_words): raise ValueError("dynamic piped interpreter consumer requires an adapter") if inline_source_index(static, language) is not None: return [] operands = piped_bash_operands(static[1:]) if language == "bash" else static[1:] if any(not word.startswith("-") and word != "-" for word in operands): return [] - text = piped_producer_text(sides[0]) + text = piped_producer_text(producer_words) if text is None: return [] payload, resolved = scan_source_payload(text, language) - line = source[: offset + sides[0][0].pos[0]].count("\n") + 1 + line = source[: offset + producer_words[0].pos[0]].count("\n") + 1 return [(line, path + "." + resolved, payload, resolved, f"{symbol}:pipe:{producer}")] diff --git a/tests/unit/scripts/test_comment_policy.py b/tests/unit/scripts/test_comment_policy.py index fc8c113c61..332af32f8c 100644 --- a/tests/unit/scripts/test_comment_policy.py +++ b/tests/unit/scripts/test_comment_policy.py @@ -1429,6 +1429,11 @@ def test_shell_executable_arguments_are_routed(source: str) -> None: ("echo -e '\\0043 hidden' | bash", "# hidden"), ("echo '# hidden' | /usr/bin/python3", "# hidden"), ('echo "# hidden" | "/opt/My Tools/python3"', "# hidden"), + (r"echo -e '\0443 hidden' | bash", "# hidden"), + (r"printf '%s\c' '# hidden' | bash", "# hidden"), + ("echo '# hidden' | env python3", "# hidden"), + ("echo '# hidden' | python3 | cat", "# hidden"), + ("echo '# hidden' 1>&1 | bash", "# hidden"), ], ) def test_piped_producer_payloads_reach_stdin_interpreters(source: str, expected: str) -> None: @@ -3482,6 +3487,7 @@ def test_mdx_imports_reach_the_typescript_scanner(source: str) -> None: ("export /* hidden */ const value = 1;", "/* hidden */"), ("import /* hidden */ './x.js';", "/* hidden */"), ("import /*\n hidden\n*/ './x.js';", "/*\n hidden\n*/"), + ("import\n React\n from 'react'; /* hidden */", "/* hidden */"), ], ) def test_mdx_esm_intertoken_comments_are_reported(source: str, comment: str) -> None: From 41e7cc42f79e2a399b6748a778ec36167dd940a5 Mon Sep 17 00:00:00 2001 From: Joe Harris Date: Sat, 10 Oct 2026 16:07:25 -0400 Subject: [PATCH 05/17] fix(comment-policy): scan wrapped interpreter payloads --- .github/workflows/ci.yml | 19 +++++++ .../audits/remediation-contract-evidence.md | 39 +++++++++++++ docs/internal/comment-policy.md | 7 ++- scripts/comment_payloads.py | 22 +++++++- tests/unit/scripts/test_comment_policy.py | 56 +++++++++++++++++++ 5 files changed, 138 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e08a6a560d..bb1f25a6d9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -896,6 +896,25 @@ jobs: - name: Install dependencies run: uv sync --group dev + - name: Set up Node.js for TypeScript comment parsing + uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 + with: + node-version: '20' + + - name: Install locked TypeScript parser + shell: bash + run: | + set -euo pipefail + package_dir="$RUNNER_TEMP/comment-policy-node" + mkdir -p "$package_dir" + cp quality/comment-policy-package.json "$package_dir/package.json" + cp quality/comment-policy-package-lock.json "$package_dir/package-lock.json" + ( + cd "$package_dir" + npm ci --ignore-scripts --no-audit --no-fund + ) + echo "COMMENT_POLICY_TYPESCRIPT=$package_dir/node_modules/typescript" >> "$GITHUB_ENV" + - name: Download medium collection evidence uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 with: diff --git a/_project/audits/remediation-contract-evidence.md b/_project/audits/remediation-contract-evidence.md index 9165203ff6..7735aec79c 100644 --- a/_project/audits/remediation-contract-evidence.md +++ b/_project/audits/remediation-contract-evidence.md @@ -375,3 +375,42 @@ The original focused scanner regressions passed 36 tests; the five additional fix groups passed 25 tests. The full policy unit file passed 716 tests with `-n 0`. `make comment-policy-check` scanned 5,749 files with zero violations and passed all 19 native syntax tests. + +### Findings D10-D12 + +- Split runners: the review's plain `sudo`, `env` and `nohup` forms already + yielded comment findings on this head. The option-bearing + `sudo -u nobody python3 -c '# explanation'` form instead returned a coverage + error. `sudo` and `nohup` now use `runner_command_start` and the registered + option parser; `env` retains its existing strict `command_words` normalization. + `test_split_runner_interpreter_arguments_are_scanned` checks eight split forms. + Unsupported `sudo` and `nohup` flags fail closed in + `test_unknown_split_runner_options_fail_closed`. +- `printf %%`: before the fix, + `printf '%%\n# explanation\n' | python3` reconstructed two percent bytes and + returned a syntax coverage error with the doubled-percent payload. The + formatter now reconstructs one percent byte. The valid Python fixture in + `test_piped_printf_escaped_percent_matches_shell_output` asserts the exact + source bytes reaching the Python scanner. +- Pipeline sides: before the fix, + `echo '# hidden' | python3 | $dynamic` and + `echo 'pass' | $interpreter` both returned no findings. The adapter now scans + the first producer/consumer pair and reports a coverage error for a dynamic + immediate consumer. The gate ignores the quoted `$$|$${key}` sed delimiter + and still recognizes unspaced shell pipes. Regressions: + `test_dynamic_later_pipeline_command_does_not_hide_interpreter_input`, + `test_dynamic_pipeline_consumer_fails_closed`, + `test_dynamic_pipeline_consumer_without_spaces_fails_closed`, and + `test_quoted_pipe_delimiter_does_not_trigger_pipeline_gate`. + +The focused D10-D12 regression selection passed 17 tests with +`BENCHBOX_SKIP_TEST_LOCK=1 uv run -- python -m pytest tests/unit/scripts/test_comment_policy.py -k 'split_runner_interpreter_arguments or unknown_split_runner_options or dynamic_pipeline_consumer or dynamic_later_pipeline_command or piped_printf_escaped_percent or quoted_pipe_delimiter' -q -n0`. +The full policy unit file passed 739 tests with +`BENCHBOX_SKIP_TEST_LOCK=1 uv run -- python -m pytest tests/unit/scripts/test_comment_policy.py -q -n0`. +`make comment-policy-check` passed: 5,776 source files, zero violations, +zero enforced failures, and all 19 native syntax tests passed. + +The medium-test job now installs the locked TypeScript package in an isolated +temporary directory and exports its module path. A local npm 11.19.1 install +with the same manifest and lockfile passed, and the MDX scanner test passed +four cases using that isolated TypeScript path. diff --git a/docs/internal/comment-policy.md b/docs/internal/comment-policy.md index 8667fee06d..6e692f7962 100644 --- a/docs/internal/comment-policy.md +++ b/docs/internal/comment-policy.md @@ -177,9 +177,10 @@ Known gaps, each a place where a comment can pass unreported: - Unknown programs that take `-c` or `-e` as data are listed in `DATA_FLAG_PROGRAMS` in `scripts/comment_execution.py`; any other program given such a flag is reported. -- Static inner commands passed as a single argument to supported runners such as - `ssh`, `watch` or `xargs` are followed; dynamically assembled commands are - not. +- Static `sudo`, `env` and `nohup` interpreter wrappers with supported options + are scanned; dynamic or unsupported wrapper options fail closed. Static inner + shell commands passed as one argument to runners such as `ssh`, `watch` or + `xargs` are followed; dynamically assembled commands are not. - HTML comments and MyST `%` lines in Markdown prose are not scanned. In the maintained docs they are copyright headers, generator start and end markers, `` markers read by `scripts/blog_content_validation.py`, diff --git a/scripts/comment_payloads.py b/scripts/comment_payloads.py index 1b764f1af6..9c4ee5b5dd 100644 --- a/scripts/comment_payloads.py +++ b/scripts/comment_payloads.py @@ -1037,6 +1037,11 @@ def shell_payloads(path: str, source: str, include_data: bool = False) -> list[t def unwrap_static_command(words: list) -> list: command = words[0].word.rsplit("/", 1)[-1] if command in SHELL_WRAPPERS or (command == "uv" and [word.word for word in words[1:3]] == ["tool", "run"]): + if command in RUNNER_COMMANDS: + skip = runner_command_start([None if word.parts else word.word for word in words], command) + if skip >= len(words): + return words + return unwrap_static_command(words[skip:]) skip = 3 if command == "uv" else 2 if command == "timeout" else 1 if command == "time": while skip < len(words) and not words[skip].parts and words[skip].word in TIME_FLAGS: @@ -1073,6 +1078,7 @@ def unwrap_static_command(words: list) -> list: r"|(? None: def runner_command_start(words: list[str | None], command: str) -> int: + if command in {"sudo", "nohup"}: + index = runner_command_index(words, command) + if any(word is None for word in words[1:index]): + raise ValueError("dynamic runner options require an adapter") + for word in words[1:index]: + if word is not None and word.startswith("-") and word not in RUNNER_VALUE_OPTIONS[command] and word != "--": + raise ValueError("unsupported runner options require an adapter") + return index if command != "find": return 1 for index, word in enumerate(words[1:], start=1): @@ -1188,6 +1202,8 @@ def runner_command_start(words: list[str | None], command: str) -> int: "nsenter", "flock", "watch", + "sudo", + "nohup", "su", "runuser", "doas", @@ -1545,7 +1561,7 @@ def piped_printf_text(args: list) -> str: expanded.append(char) index += 1 elif format_raw[index : index + 2] == "%%": - expanded.append("%%") + expanded.append("%") index += 2 elif format_raw[index : index + 2] == "%s": val = values[value_index] if value_index < len(values) else "" @@ -1618,10 +1634,12 @@ def piped_sides(node: bashlex.ast.node) -> list | None: if len(commands) < 2: return None sides = [[part for part in command.parts if part.kind == "word"] for command in commands] - if any(not side or side[0].parts for side in sides): + if not sides[0] or not sides[1] or sides[0][0].parts: return None if sides[0][0].word.rsplit("/", 1)[-1] not in PIPED_PRODUCERS: return None + if sides[1][0].parts: + raise ValueError("dynamic piped interpreter consumer requires an adapter") unwrapped_consumer = sides[1] if not any(w.parts for w in sides[1]): head_cmd = sides[1][0].word.rsplit("/", 1)[-1] diff --git a/tests/unit/scripts/test_comment_policy.py b/tests/unit/scripts/test_comment_policy.py index 332af32f8c..3651174c4a 100644 --- a/tests/unit/scripts/test_comment_policy.py +++ b/tests/unit/scripts/test_comment_policy.py @@ -1415,6 +1415,62 @@ def test_shell_executable_arguments_are_routed(source: str) -> None: assert [f.text for f in scan("a.sh", source, "bash")] == ["# explanation"] +@pytest.mark.parametrize( + "source", + [ + "sudo python3 -c '# explanation'", + "sudo -u nobody python3 -c '# explanation'", + "sudo --user nobody python3 -c '# explanation'", + "env python3 -c '# explanation'", + "env POLICY_TEST=1 python3 -c '# explanation'", + "env -i POLICY_TEST=1 python3 -c '# explanation'", + "nohup python3 -c '# explanation'", + "nohup -- python3 -c '# explanation'", + ], +) +def test_split_runner_interpreter_arguments_are_scanned(source: str) -> None: + assert [finding.text for finding in scan("a.sh", source, "bash")] == ["# explanation"] + + +@pytest.mark.parametrize( + "source", + [ + "sudo --unknown value python3 -c '# explanation'", + "nohup -x python3 -c '# explanation'", + ], +) +def test_unknown_split_runner_options_fail_closed(source: str) -> None: + assert [finding.kind for finding in scan("a.sh", source, "bash")] == ["coverage-error"] + + +@pytest.mark.parametrize("consumer", ["$interpreter", "${INTERPRETER}", '"$1"']) +def test_dynamic_pipeline_consumer_fails_closed(consumer: str) -> None: + findings = scan("a.sh", f"echo 'pass' | {consumer}", "bash") + assert [finding.kind for finding in findings] == ["coverage-error"] + + +def test_dynamic_pipeline_consumer_without_spaces_fails_closed() -> None: + findings = scan("a.sh", "echo 'pass'|$interpreter", "bash") + assert [finding.kind for finding in findings] == ["coverage-error"] + + +def test_quoted_pipe_delimiter_does_not_trigger_pipeline_gate() -> None: + source = 'sed -i -E "s|^[[:space:]]*$${key}[[:space:]]*=.*$$|$${key} = $${value}|" "$$file"' + assert scan("a.sh", source, "bash") == [] + + +def test_dynamic_later_pipeline_command_does_not_hide_interpreter_input() -> None: + findings = scan("a.sh", "echo '# hidden' | python3 | $dynamic", "bash") + assert [(finding.kind, finding.text) for finding in findings] == [("comment", "# hidden")] + + +def test_piped_printf_escaped_percent_matches_shell_output() -> None: + findings = scan("a.sh", "printf 'print(\"%%\")\\n# explanation\\n' | python3", "bash") + assert [(finding.kind, finding.text, finding.payload) for finding in findings] == [ + ("comment", "# explanation", 'print("%")\n# explanation\n') + ] + + @pytest.mark.parametrize( ("source", "expected"), [ From 24c0bc9f516de96a8c171f9b3a13db8ba5f338fc Mon Sep 17 00:00:00 2001 From: Joe Harris Date: Sat, 10 Oct 2026 21:50:01 -0400 Subject: [PATCH 06/17] fix(comment-policy): close runner and MDX scan gaps --- .../audits/remediation-contract-evidence.md | 42 ++++++ docs/internal/comment-policy.md | 7 +- scripts/comment_payloads.py | 121 +++++++++++++++--- tests/unit/scripts/test_comment_policy.py | 44 ++++++- 4 files changed, 189 insertions(+), 25 deletions(-) diff --git a/_project/audits/remediation-contract-evidence.md b/_project/audits/remediation-contract-evidence.md index 7735aec79c..44cd388df6 100644 --- a/_project/audits/remediation-contract-evidence.md +++ b/_project/audits/remediation-contract-evidence.md @@ -414,3 +414,45 @@ The medium-test job now installs the locked TypeScript package in an isolated temporary directory and exports its module path. A local npm 11.19.1 install with the same manifest and lockfile passed, and the MDX scanner test passed four cases using that isolated TypeScript path. + +### Findings D13-D16 + +- D13, runner-wrapped consumers: positive cases in + `test_piped_producer_payloads_reach_stdin_interpreters` cover + `sudo python3`, `sudo -u nobody python3`, `nohup python3`, and + `nohup -- python3`. Negative controls in + `test_unknown_split_runner_options_fail_closed` cover `sudo --unknown` and + `nohup -x`, both as direct commands and piped consumers. +- D14, multi-hop pipelines: the same positive test covers + `echo '# hidden' | cat | python3` and a two-`cat` chain. The dynamic + consumer case `test_dynamic_pipeline_consumer_fails_closed[cat | $interpreter]` + fails closed. `test_piped_unsupported_intermediate_before_interpreter_fails_closed` + covers both `sed ... | python3` and `sed ... | $dynamic`. +- D15, MDX inline code: `test_mdx_jsx_comments_ignore_inline_code` checks + single-backtick, double-backtick, multiline and escaped-opener forms. It + rejects JSX-shaped text inside code spans and preserves a real adjacent + JSX comment. +- D16, TypeScript flow coverage: `test_reviewed_javascript_flows_still_occur` + now fails if Node.js or TypeScript is unavailable. The `medium-test` job + installs the locked parser before running that test. The separate + `comment-policy` job's trusted `run_comment_policy.py` parser environment + also installs the locked package and exports `COMMENT_POLICY_TYPESCRIPT`; + each job retains the setup its consumer requires. + +The focused regression selection passed 61 tests with +`uv run -- pytest -q tests/unit/scripts/test_comment_policy.py -k 'piped_ or dynamic_pipeline_consumer or mdx_jsx_comments or reviewed_javascript_flows_still_occur'`. + +The full policy unit file passed 754 tests with +`uv run -- pytest -q tests/unit/scripts/test_comment_policy.py -n0`. +The missing-parser control +`env COMMENT_POLICY_TYPESCRIPT=/tmp/comment-policy-typescript-missing uv run -- pytest -q tests/unit/scripts/test_comment_policy.py -k reviewed_javascript_flows_still_occur -n0` +failed as expected with one assertion, proving the CI test cannot silently +skip the TypeScript flow check. + +The final `make comment-policy-check` passed: 5,776 source files, zero +violations, zero enforced failures, and all 19 native syntax tests passed. +Three final runs measured 269.54, 298.41, and 308.01 seconds wall. Prior paired +candidate runs measured 252.23-295.88 seconds; the latter two final runs were +2.53 and 12.13 seconds above that range. Earlier paired runs varied widely. +These whole-gate timings do not isolate scanner performance; no explicit +wall-time budget is set. diff --git a/docs/internal/comment-policy.md b/docs/internal/comment-policy.md index 6e692f7962..a582da4e02 100644 --- a/docs/internal/comment-policy.md +++ b/docs/internal/comment-policy.md @@ -144,9 +144,10 @@ Python is read with its AST and tokenizer, including standalone strings and runtime docstring assignments; JavaScript and TypeScript with the isolated TypeScript parser; SQL-valued strings and execution-sink arguments as SQL; other formats with Pygments lexers; embedded code with its own language check. -Shell heredocs and literal `echo`/`printf` pipelines into supported stdin -interpreters are scanned; MDX top-level ESM and JSX block comments are -extracted. +Shell heredocs and literal `echo`/`printf` pipelines through plain `cat` stages +or supported `sudo`, `env` and `nohup` wrappers into supported stdin +interpreters are scanned. MDX top-level ESM and JSX block comments outside +Markdown code spans and fences are extracted. Executable paths are matched by basename; consumer output redirects preserve piped input, while redirected stdin fails closed. Unknown input is a coverage error, never a pass, and under blocking enforcement diff --git a/scripts/comment_payloads.py b/scripts/comment_payloads.py index 9c4ee5b5dd..540766e977 100644 --- a/scripts/comment_payloads.py +++ b/scripts/comment_payloads.py @@ -410,12 +410,57 @@ def mdx_js_sources(path: str, source: str) -> list[tuple[int, str, str, str, str return result +def mdx_inline_code_mask(source: str) -> str: + masked = list(source) + index = 0 + while index < len(source): + if source[index] != "`": + index += 1 + continue + run_end = index + 1 + while run_end < len(source) and source[run_end] == "`": + run_end += 1 + backslash_count = 0 + previous = index - 1 + while previous >= 0 and source[previous] == "\\": + backslash_count += 1 + previous -= 1 + if backslash_count % 2: + index = run_end + continue + delimiter = source[index:run_end] + closing = source.find(delimiter, run_end) + while closing >= 0: + closing_end = closing + len(delimiter) + part_of_longer_run = (closing > 0 and source[closing - 1] == "`") or ( + closing_end < len(source) and source[closing_end] == "`" + ) + if part_of_longer_run: + closing = source.find(delimiter, closing_end) + continue + if "\0" in source[run_end:closing]: + closing = -1 + break + for position in range(index, closing_end): + if source[position] not in {"\n", "\0"}: + masked[position] = " " + index = closing_end + break + else: + index = run_end + continue + if closing < 0: + index = run_end + return "".join(masked) + + def mdx_jsx_comments(source: str) -> list[tuple[int, str]]: lines = mdx_masked_lines(source) for start, _, text, _, _ in mdx_js_sources(path="", source=source): for number in range(start - 1, start - 1 + len(text.splitlines())): lines[number] = None - prose = "\n".join("" if line is None else line for line in lines) + prose = "\n".join("\0" if line is None else line for line in lines) + prose = mdx_inline_code_mask(prose) return [ (prose[: match.start()].count("\n") + 1, match.group().rstrip()) for match in MDX_JSX_COMMENT.finditer(prose) ] @@ -1611,6 +1656,7 @@ def piped_redirect_effect(commands: list) -> tuple[bool, bool]: stdin_redirects = {"<", "<<", "<<-", "<<<", "<>", "<&"} producer_stdout_redirected = False consumer_stdin_redirected = False + consumer_index = len(commands) - 1 for command_index, command in enumerate(commands): for part in command.parts: if part.kind != "redirect": @@ -1622,11 +1668,51 @@ def piped_redirect_effect(commands: list) -> tuple[bool, bool]: output_target = getattr(part, "output", None) if not (part.type == ">&" and output_target == 1): producer_stdout_redirected = True - if command_index == 1 and descriptor == 0: + if command_index == consumer_index and descriptor == 0: consumer_stdin_redirected = True return producer_stdout_redirected, consumer_stdin_redirected +def piped_consumer_words(words: list) -> tuple[list, str]: + command = words[0].word.rsplit("/", 1)[-1] + if command in {"sudo", "nohup"}: + unwrapped, command = unwrap_runner_command(words, command) + if unwrapped is words and any(word.parts for word in words[1:]): + raise ValueError("dynamic piped interpreter consumer requires an adapter") + return unwrapped, command + if command == "env" and len(words) > 1: + index = 1 + while index < len(words): + word = words[index] + if word.parts: + raise ValueError("dynamic piped interpreter consumer requires an adapter") + if re.match(r"[A-Za-z_][A-Za-z0-9_]*=", word.word) or word.word in {"-i", "--ignore-environment"}: + index += 1 + continue + break + if index < len(words) and not words[index].word.startswith("-"): + unwrapped = words[index:] + return unwrapped, unwrapped[0].word.rsplit("/", 1)[-1] + return words, command + + +def piped_passthrough_cat(command: bashlex.ast.node, words: list) -> bool: + if not words or words[0].word.rsplit("/", 1)[-1] != "cat": + return False + if any(part.kind == "redirect" for part in command.parts) or any(word.parts for word in words): + return False + return len(words) == 1 or len(words) == 2 and words[1].word == "-" + + +def piped_may_execute_interpreter(side: list) -> bool: + if not side: + return False + if side[0].parts: + return True + _, command = piped_consumer_words(side) + return piped_consumer_language(command) is not None or command in SHELL_UNMODELED + + def piped_sides(node: bashlex.ast.node) -> list | None: if node.kind != "pipeline": return None @@ -1634,25 +1720,24 @@ def piped_sides(node: bashlex.ast.node) -> list | None: if len(commands) < 2: return None sides = [[part for part in command.parts if part.kind == "word"] for command in commands] - if not sides[0] or not sides[1] or sides[0][0].parts: + if not sides[0] or sides[0][0].parts: return None if sides[0][0].word.rsplit("/", 1)[-1] not in PIPED_PRODUCERS: return None - if sides[1][0].parts: - raise ValueError("dynamic piped interpreter consumer requires an adapter") - unwrapped_consumer = sides[1] - if not any(w.parts for w in sides[1]): - head_cmd = sides[1][0].word.rsplit("/", 1)[-1] - if head_cmd == "env" and len(sides[1]) > 1: - idx = 1 - while idx < len(sides[1]) and ( - re.match(r"[A-Za-z_][A-Za-z0-9_]*=", sides[1][idx].word) - or sides[1][idx].word in {"-i", "--ignore-environment"} - ): - idx += 1 - if idx < len(sides[1]) and not sides[1][idx].word.startswith("-"): - unwrapped_consumer = sides[1][idx:] - return [sides[0], unwrapped_consumer, commands[:2]] + for index, side in enumerate(sides[1:], start=1): + if not side: + raise ValueError("empty piped interpreter consumer requires an adapter") + if side[0].parts: + raise ValueError("dynamic piped interpreter consumer requires an adapter") + if piped_passthrough_cat(commands[index], side): + continue + consumer_words, consumer = piped_consumer_words(side) + if piped_consumer_language(consumer) is not None or consumer in SHELL_UNMODELED: + return [sides[0], consumer_words, commands[: index + 1]] + if any(piped_may_execute_interpreter(later) for later in sides[index + 1 :]): + raise ValueError("piped interpreter input through an unsupported command requires an adapter") + return [sides[0], consumer_words, commands[: index + 1]] + return None def piped_unmodeled_check(side: list) -> None: diff --git a/tests/unit/scripts/test_comment_policy.py b/tests/unit/scripts/test_comment_policy.py index 3651174c4a..d026a6d8c8 100644 --- a/tests/unit/scripts/test_comment_policy.py +++ b/tests/unit/scripts/test_comment_policy.py @@ -1437,13 +1437,18 @@ def test_split_runner_interpreter_arguments_are_scanned(source: str) -> None: [ "sudo --unknown value python3 -c '# explanation'", "nohup -x python3 -c '# explanation'", + "echo '# hidden' | sudo --unknown value python3", + "echo '# hidden' | nohup -x python3", ], ) def test_unknown_split_runner_options_fail_closed(source: str) -> None: assert [finding.kind for finding in scan("a.sh", source, "bash")] == ["coverage-error"] -@pytest.mark.parametrize("consumer", ["$interpreter", "${INTERPRETER}", '"$1"']) +@pytest.mark.parametrize( + "consumer", + ["$interpreter", "${INTERPRETER}", '"$1"', "cat | $interpreter"], +) def test_dynamic_pipeline_consumer_fails_closed(consumer: str) -> None: findings = scan("a.sh", f"echo 'pass' | {consumer}", "bash") assert [finding.kind for finding in findings] == ["coverage-error"] @@ -1464,6 +1469,18 @@ def test_dynamic_later_pipeline_command_does_not_hide_interpreter_input() -> Non assert [(finding.kind, finding.text) for finding in findings] == [("comment", "# hidden")] +@pytest.mark.parametrize( + "source", + [ + "echo '# hidden' | sed 's/x/y/' | python3", + "echo '# hidden' | sed 's/x/y/' | $dynamic", + ], +) +def test_piped_unsupported_intermediate_before_interpreter_fails_closed(source: str) -> None: + findings = scan("a.sh", source, "bash") + assert [finding.kind for finding in findings] == ["coverage-error"] + + def test_piped_printf_escaped_percent_matches_shell_output() -> None: findings = scan("a.sh", "printf 'print(\"%%\")\\n# explanation\\n' | python3", "bash") assert [(finding.kind, finding.text, finding.payload) for finding in findings] == [ @@ -1488,6 +1505,12 @@ def test_piped_printf_escaped_percent_matches_shell_output() -> None: (r"echo -e '\0443 hidden' | bash", "# hidden"), (r"printf '%s\c' '# hidden' | bash", "# hidden"), ("echo '# hidden' | env python3", "# hidden"), + ("echo '# hidden' | sudo python3", "# hidden"), + ("echo '# hidden' | sudo -u nobody python3", "# hidden"), + ("echo '# hidden' | nohup python3", "# hidden"), + ("echo '# hidden' | nohup -- python3", "# hidden"), + ("echo '# hidden' | cat | python3", "# hidden"), + ("echo '# hidden' | cat | cat | python3", "# hidden"), ("echo '# hidden' | python3 | cat", "# hidden"), ("echo '# hidden' 1>&1 | bash", "# hidden"), ], @@ -3009,15 +3032,14 @@ def test_reviewed_javascript_flows_are_exact_and_current() -> None: def test_reviewed_javascript_flows_still_occur() -> None: import json - import os import shutil import subprocess from comment_syntax import REVIEWED_JAVASCRIPT_FLOWS, resolve_typescript_dir typescript = resolve_typescript_dir(ROOT) - if shutil.which("node") is None or typescript is None or not typescript.exists(): - pytest.skip("the TypeScript package for scripts/comment_syntax_js.cjs is not installed") + assert shutil.which("node") is not None, "Node.js is required to verify reviewed JavaScript flows" + assert typescript is not None and typescript.exists(), "the TypeScript parser is required for reviewed flows" paths = sorted({path for path, _ in REVIEWED_JAVASCRIPT_FLOWS}) requests = {path: (ROOT / path).read_text(encoding="utf-8") for path in paths} result = subprocess.run( @@ -3492,6 +3514,20 @@ def test_mdx_jsx_comments_are_found(source: str, expected: list[tuple[int, str]] assert [(f.kind, f.line, f.text) for f in findings] == [("comment", *item) for item in expected] +@pytest.mark.parametrize( + ("source", "expected"), + [ + ("`{/* inline */}`", []), + ("``{/* inline */}``", []), + ("`{/* inline\ncomment */}`\n{/* visible */}", [(3, "{/* visible */}")]), + (r"\`{/* visible */}`", [(1, "{/* visible */}")]), + ], +) +def test_mdx_jsx_comments_ignore_inline_code(source: str, expected: list[tuple[int, str]]) -> None: + findings = scan("website/src/content/docs/page.mdx", source, "mdx", {}) + assert [(finding.line, finding.text) for finding in findings] == expected + + def test_mdx_prose_and_fences_are_scanned_once() -> None: source = "```python\n# explanation\n```\n\n{/* prose */}\n" findings = scan("website/src/content/docs/page.mdx", source, "mdx", {}) From cacbff9f340e2e0ee8e0b7d2002bbc5faeb9d503 Mon Sep 17 00:00:00 2001 From: Joe Harris Date: Sat, 10 Oct 2026 22:03:26 -0400 Subject: [PATCH 07/17] docs(comment-policy): record rebased gate evidence --- _project/audits/remediation-contract-evidence.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/_project/audits/remediation-contract-evidence.md b/_project/audits/remediation-contract-evidence.md index 44cd388df6..46d1eba2eb 100644 --- a/_project/audits/remediation-contract-evidence.md +++ b/_project/audits/remediation-contract-evidence.md @@ -449,10 +449,10 @@ The missing-parser control failed as expected with one assertion, proving the CI test cannot silently skip the TypeScript flow check. -The final `make comment-policy-check` passed: 5,776 source files, zero +After rebasing, `make comment-policy-check` passed: 5,779 files, zero violations, zero enforced failures, and all 19 native syntax tests passed. -Three final runs measured 269.54, 298.41, and 308.01 seconds wall. Prior paired -candidate runs measured 252.23-295.88 seconds; the latter two final runs were -2.53 and 12.13 seconds above that range. Earlier paired runs varied widely. -These whole-gate timings do not isolate scanner performance; no explicit -wall-time budget is set. +`/usr/bin/time -p` measured 273.46 seconds wall, 230.01 user, and 33.32 sys. +That run falls within the earlier candidate wall-time range of 252.23-295.88 +seconds and below the historical 358 seconds wall / 276 seconds user baseline. +Three earlier final runs measured 269.54, 298.41, and 308.01 seconds wall, +showing run-to-run variance. No explicit gate-time budget is set. From 0429b866258e9298c71344bd2dbddc8a65cd940d Mon Sep 17 00:00:00 2001 From: Joe Harris Date: Sat, 10 Oct 2026 22:59:12 -0400 Subject: [PATCH 08/17] fix(comment-policy): close scanner review gaps --- .../audits/remediation-contract-evidence.md | 64 +++++++++++++++---- docs/operations/ci-local-parity.md | 4 ++ scripts/comment_payloads.py | 53 +++++++++++---- tests/system/test_ci_lint_parity.py | 1 + tests/unit/scripts/test_comment_policy.py | 11 +++- 5 files changed, 109 insertions(+), 24 deletions(-) diff --git a/_project/audits/remediation-contract-evidence.md b/_project/audits/remediation-contract-evidence.md index 46d1eba2eb..7a221883e5 100644 --- a/_project/audits/remediation-contract-evidence.md +++ b/_project/audits/remediation-contract-evidence.md @@ -419,10 +419,10 @@ four cases using that isolated TypeScript path. - D13, runner-wrapped consumers: positive cases in `test_piped_producer_payloads_reach_stdin_interpreters` cover - `sudo python3`, `sudo -u nobody python3`, `nohup python3`, and - `nohup -- python3`. Negative controls in - `test_unknown_split_runner_options_fail_closed` cover `sudo --unknown` and - `nohup -x`, both as direct commands and piped consumers. + `sudo python3`, `sudo -u nobody python3`, `nohup python3`, + `nohup -- python3`, and `env` with `-u`, `--unset`, and `-C` options. + Negative controls in `test_unknown_split_runner_options_fail_closed` cover + unsupported `sudo`, `nohup`, and `env` options, including `env -S`. - D14, multi-hop pipelines: the same positive test covers `echo '# hidden' | cat | python3` and a two-`cat` chain. The dynamic consumer case `test_dynamic_pipeline_consumer_fails_closed[cat | $interpreter]` @@ -442,17 +442,59 @@ four cases using that isolated TypeScript path. The focused regression selection passed 61 tests with `uv run -- pytest -q tests/unit/scripts/test_comment_policy.py -k 'piped_ or dynamic_pipeline_consumer or mdx_jsx_comments or reviewed_javascript_flows_still_occur'`. -The full policy unit file passed 754 tests with +The full policy unit file passed 759 tests with `uv run -- pytest -q tests/unit/scripts/test_comment_policy.py -n0`. The missing-parser control `env COMMENT_POLICY_TYPESCRIPT=/tmp/comment-policy-typescript-missing uv run -- pytest -q tests/unit/scripts/test_comment_policy.py -k reviewed_javascript_flows_still_occur -n0` failed as expected with one assertion, proving the CI test cannot silently skip the TypeScript flow check. -After rebasing, `make comment-policy-check` passed: 5,779 files, zero +After the env-option fix, `make comment-policy-check` passed: 5,779 files, zero violations, zero enforced failures, and all 19 native syntax tests passed. -`/usr/bin/time -p` measured 273.46 seconds wall, 230.01 user, and 33.32 sys. -That run falls within the earlier candidate wall-time range of 252.23-295.88 -seconds and below the historical 358 seconds wall / 276 seconds user baseline. -Three earlier final runs measured 269.54, 298.41, and 308.01 seconds wall, -showing run-to-run variance. No explicit gate-time budget is set. +Two timed runs measured 483.21 seconds wall / 319.96 user / 63.34 sys and +321.52 / 263.31 / 38.95. The latest run was below the historical 358-second +wall / 276-second user baseline; the first exceeded it. An earlier post-rebase +run before the env-option fix measured 273.46 / 230.01 / 33.32. Earlier paired +candidate runs ranged from 252.23 to 295.88 seconds wall. Run-to-run variance +is large; the measurements do not isolate scanner performance, and no explicit +gate-time budget is set. + +### Findings D17-D19 and CI parity + +- D17, `env`-wrapped pipeline consumers: `piped_consumer_words` unwraps the + registered `env` value and flag options before identifying the interpreter. + `test_piped_producer_payloads_reach_stdin_interpreters` covers `-u`, + `--unset`, and `-C`; `test_unknown_split_runner_options_fail_closed` rejects + unknown options and `-S`/`--split-string`. +- D18, unparseable piped consumers: the fallback now treats a non-heredoc pipe + to a modeled interpreter, or a dynamic pipe command, as comment-capable and + raises a coverage error when parsing cannot continue. The positive + `echo x | python3 )` and negative `echo x | cat )` plus quoted-pipe controls + are in `test_unparseable_shell_chunks_fail_closed_only_when_they_can_hold_a_comment`. + `test_piped_heredoc_is_executable` confirms heredoc payloads remain with the + dedicated shell payload adapter. +- D19, whitespace-padded MDX JSX comments: `MDX_JSX_COMMENT` accepts + whitespace between `{` and `/*`. `test_mdx_jsx_comments_are_found` requires + the spaced comment; `test_mdx_jsx_comments_ignore_inline_code` rejects the + same shape inside a code span. +- The CI parity regression lacked a local equivalent for the medium-test + locked TypeScript parser installation. `MERGE_GATE_LOCAL_EQUIVALENTS` maps + that step to `test-medium`, and `docs/operations/ci-local-parity.md` records + the local command. + +The focused D17-D19 regression selection passed 47 tests: + +```bash +uv run -- python -m pytest tests/unit/scripts/test_comment_policy.py \ + -k 'piped_producer_payloads_reach_stdin_interpreters or unknown_split_runner_options_fail_closed or unparseable_shell_chunks_fail_closed_only_when_they_can_hold_a_comment or mdx_jsx_comments_are_found or mdx_jsx_comments_ignore_inline_code' \ + -q -n0 +``` + +The full policy unit file passed 763 tests with +`uv run -- python -m pytest tests/unit/scripts/test_comment_policy.py -q -n0`. +The two affected CI parity tests passed with +`uv run -- python -m pytest tests/system/test_ci_lint_parity.py::test_non_lint_merge_gate_guards_have_local_equivalent_or_documented_exemption tests/system/test_ci_lint_parity.py::test_merge_gate_local_equivalents_and_exemptions_are_documented -q -n0`. +Ruff check and format checks passed for all three changed Python files. + +`make comment-policy-check` passed: 5,779 source files, zero violations, zero +enforced failures, and all 19 native syntax tests passed. diff --git a/docs/operations/ci-local-parity.md b/docs/operations/ci-local-parity.md index 15ff3244bf..6074cd1bfc 100644 --- a/docs/operations/ci-local-parity.md +++ b/docs/operations/ci-local-parity.md @@ -154,6 +154,10 @@ merge-gating workflows. Each such step must name a local equivalent or carry a written reason in the test's `MERGE_GATE_EXEMPTIONS` table, so a new check step cannot become a silent CI-only failure. +The `medium-test` job installs the locked TypeScript parser used by +`tests/unit/scripts/test_comment_policy.py`; `make test-medium` runs the same +reviewed-JavaScript flow test locally. + The `code-test` step that runs changed tests on the curated release tree has a local equivalent: `uv run -- python scripts/release_curation_dry_run.py --changed-since origin/develop`. diff --git a/scripts/comment_payloads.py b/scripts/comment_payloads.py index 540766e977..b8cd30a2b7 100644 --- a/scripts/comment_payloads.py +++ b/scripts/comment_payloads.py @@ -321,7 +321,7 @@ def astro_expression_sources(path: str, source: str) -> list[tuple[int, str, str ] -MDX_JSX_COMMENT = re.compile(r"\{/\*.*?\*/\s*\}", re.S) +MDX_JSX_COMMENT = re.compile(r"\{\s*/\*.*?\*/\s*\}", re.S) MDX_STATEMENT_START = re.compile(r"(?:import|export)(?=\s|$)") MDX_IMPORT_FROM = re.compile(r"\bfrom\s+['\"][^'\"\n]+['\"]") MDX_IMPORT_BARE = re.compile(r"^import\s+['\"]") @@ -1116,15 +1116,22 @@ def unwrap_static_command(words: list) -> list: return words +SHELL_DYNAMIC_PIPED_COMMAND = r"""(? int: "nohup": set(), } +PIPED_ENV_FLAGS = {"-i", "--ignore-environment", "-0", "--null"} +PIPED_ENV_VALUE_OPTIONS = RUNNER_VALUE_OPTIONS["env"] - {"-S", "--split-string"} +PIPED_ENV_ASSIGNMENT = re.compile(r"[A-Za-z_][A-Za-z0-9_]*=.*") + def runner_command_index(words: list[str | None], command: str) -> int: value_options = RUNNER_VALUE_OPTIONS.get(command, set()) @@ -1517,7 +1528,9 @@ def neutral_list_operators(text: str) -> str: def chunk_can_hold_comment(chunk: str) -> bool: - if SHELL_MODELED_INLINE.search(chunk): + if SHELL_MODELED_INLINE.search(chunk) or ( + not re.search(r"(? tuple[list, str]: while index < len(words): word = words[index] if word.parts: - raise ValueError("dynamic piped interpreter consumer requires an adapter") - if re.match(r"[A-Za-z_][A-Za-z0-9_]*=", word.word) or word.word in {"-i", "--ignore-environment"}: + break + value = word.word + if value in PIPED_ENV_FLAGS or PIPED_ENV_ASSIGNMENT.fullmatch(value): index += 1 continue - break - if index < len(words) and not words[index].word.startswith("-"): - unwrapped = words[index:] - return unwrapped, unwrapped[0].word.rsplit("/", 1)[-1] + if value.startswith("-"): + break + return words[index:], value.rsplit("/", 1)[-1] + static = [None if word.parts else word.word for word in words[1:]] + tail = _wrapper_tail( + static, + value_options=PIPED_ENV_VALUE_OPTIONS, + flag_options=PIPED_ENV_FLAGS, + wrapper="piped env", + ) + while tail and tail[0] is not None and PIPED_ENV_ASSIGNMENT.fullmatch(tail[0]): + tail = tail[1:] + if not tail: + raise ValueError("piped env executable requires an adapter") + consumed = len(static) - len(tail) + unwrapped = words[consumed + 1 :] + if not unwrapped or unwrapped[0].parts: + raise ValueError("dynamic piped interpreter consumer requires an adapter") + return unwrapped, unwrapped[0].word.rsplit("/", 1)[-1] return words, command diff --git a/tests/system/test_ci_lint_parity.py b/tests/system/test_ci_lint_parity.py index 48013fade0..97d86cbbb5 100644 --- a/tests/system/test_ci_lint_parity.py +++ b/tests/system/test_ci_lint_parity.py @@ -67,6 +67,7 @@ ("ci.yml", "ci-paths", "Check release content"): "release-check", ("ci.yml", "code-test", "Run changed tests on the curated release tree"): "scripts/release_curation_dry_run.py", ("ci.yml", "comment-policy", "Enforce comment and docstring policy"): "comment-policy-check", + ("ci.yml", "medium-test", "Install locked TypeScript parser"): "test-medium", ("ci.yml", "site-build", "Typecheck, audit and build website"): "site-check", ("ci.yml", "site-build", "Test built website and not-found fallback"): "site-test-built", ("ci.yml", "public-site-visual-astro-dry-run", "Capture public site from the Astro build"): "site-visual-capture", diff --git a/tests/unit/scripts/test_comment_policy.py b/tests/unit/scripts/test_comment_policy.py index d026a6d8c8..7ebb433c6d 100644 --- a/tests/unit/scripts/test_comment_policy.py +++ b/tests/unit/scripts/test_comment_policy.py @@ -1439,6 +1439,8 @@ def test_split_runner_interpreter_arguments_are_scanned(source: str) -> None: "nohup -x python3 -c '# explanation'", "echo '# hidden' | sudo --unknown value python3", "echo '# hidden' | nohup -x python3", + "echo '# hidden' | env --unknown python3", + "echo '# hidden' | env -S python3", ], ) def test_unknown_split_runner_options_fail_closed(source: str) -> None: @@ -1505,6 +1507,9 @@ def test_piped_printf_escaped_percent_matches_shell_output() -> None: (r"echo -e '\0443 hidden' | bash", "# hidden"), (r"printf '%s\c' '# hidden' | bash", "# hidden"), ("echo '# hidden' | env python3", "# hidden"), + ("echo '# hidden' | env -u HOME python3", "# hidden"), + ("echo '# hidden' | env --unset=HOME python3", "# hidden"), + ("echo '# hidden' | env -C /tmp python3", "# hidden"), ("echo '# hidden' | sudo python3", "# hidden"), ("echo '# hidden' | sudo -u nobody python3", "# hidden"), ("echo '# hidden' | nohup python3", "# hidden"), @@ -3401,8 +3406,10 @@ def test_every_astro_expression_is_sent_to_the_typescript_scanner() -> None: ('LEVELS=($(printf "%s\\n" 1 2 | awk -v max="$MAX" \'$1 <= max\' | sort -nu))\n', []), ('if ! awk "BEGIN { exit !($s >= 1) }"; then\n exit 1\nfi\n', ["coverage-error"]), ("if ! awk -v s=\"$s\" 'BEGIN { exit !(s >= 1) }'; then\n exit 1\nfi\n", []), - ("LEVELS=($(printf x | awk '{print} # note'))\n", ["coverage-error"]), ("LEVELS=($(psql -c 'SELECT 1 -- note'))\n", ["coverage-error"]), + ("echo x | python3 )\n", ["coverage-error"]), + ("echo x | cat )\n", []), + ("echo '| python3'\n", []), ], ) def test_unparseable_shell_chunks_fail_closed_only_when_they_can_hold_a_comment(source: str, kinds: list) -> None: @@ -3504,6 +3511,7 @@ def test_mdx_pages_scan_prose_fences_and_imports() -> None: ("source", "expected"), [ ("# Title\n\n{/* explanation */}\n", [(3, "{/* explanation */}")]), + ("{ /* explanation */ }\n", [(1, "{ /* explanation */ }")]), ("{/* first */}\n\n{/* second */}\n", [(1, "{/* first */}"), (3, "{/* second */}")]), ("
\n {/* explanation */}\n
\n", [(2, "{/* explanation */}")]), ("{/* multi\nline */}\n", [(1, "{/* multi\nline */}")]), @@ -3518,6 +3526,7 @@ def test_mdx_jsx_comments_are_found(source: str, expected: list[tuple[int, str]] ("source", "expected"), [ ("`{/* inline */}`", []), + ("`{ /* inline */ }`", []), ("``{/* inline */}``", []), ("`{/* inline\ncomment */}`\n{/* visible */}", [(3, "{/* visible */}")]), (r"\`{/* visible */}`", [(1, "{/* visible */}")]), From 50183db50c15b46a1dbd474d3bf7cd297f1c2975 Mon Sep 17 00:00:00 2001 From: Joe Harris Date: Sat, 10 Oct 2026 23:12:23 -0400 Subject: [PATCH 09/17] docs(comment-policy): record final gate timings --- _project/audits/remediation-contract-evidence.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/_project/audits/remediation-contract-evidence.md b/_project/audits/remediation-contract-evidence.md index 7a221883e5..506f1d434e 100644 --- a/_project/audits/remediation-contract-evidence.md +++ b/_project/audits/remediation-contract-evidence.md @@ -498,3 +498,10 @@ Ruff check and format checks passed for all three changed Python files. `make comment-policy-check` passed: 5,779 source files, zero violations, zero enforced failures, and all 19 native syntax tests passed. + +The latest timed gate run measured 690.49 seconds wall, 375.00 user, and 61.54 +sys, above the historical 358-second wall / 276-second user baseline. Earlier +runs after env-option support ranged from 321.52 to 483.21 seconds wall and +263.31 to 319.96 user. These whole-gate timings use the immutable base checker +in `scripts/run_comment_policy.py`; they do not isolate candidate scanner cost +or supersede the paired scanner measurements recorded above. From 5c33b6075990429f875b441bc43665f6a1e1a991 Mon Sep 17 00:00:00 2001 From: Joe Harris Date: Sun, 11 Oct 2026 07:32:46 -0400 Subject: [PATCH 10/17] fix(comment-policy): close wrapped pipeline gaps --- docs/internal/comment-policy.md | 16 ++- scripts/comment_payloads.py | 139 +++++++++++++++++++--- tests/unit/scripts/test_comment_policy.py | 54 ++++++++- 3 files changed, 187 insertions(+), 22 deletions(-) diff --git a/docs/internal/comment-policy.md b/docs/internal/comment-policy.md index a582da4e02..8aea0e4a46 100644 --- a/docs/internal/comment-policy.md +++ b/docs/internal/comment-policy.md @@ -146,8 +146,11 @@ TypeScript parser; SQL-valued strings and execution-sink arguments as SQL; other formats with Pygments lexers; embedded code with its own language check. Shell heredocs and literal `echo`/`printf` pipelines through plain `cat` stages or supported `sudo`, `env` and `nohup` wrappers into supported stdin -interpreters are scanned. MDX top-level ESM and JSX block comments outside -Markdown code spans and fences are extracted. +interpreters are scanned. Static `sudo`/`env` producer wrappers are also +scanned. Static SSH pipelines into supported remote stdin +interpreters are also scanned; `watch` and `xargs` pipelines to stdin +interpreters fail closed when input routing is ambiguous. MDX top-level ESM +and JSX block comments outside Markdown code spans and fences are extracted. Executable paths are matched by basename; consumer output redirects preserve piped input, while redirected stdin fails closed. Unknown input is a coverage error, never a pass, and under blocking enforcement @@ -178,10 +181,11 @@ Known gaps, each a place where a comment can pass unreported: - Unknown programs that take `-c` or `-e` as data are listed in `DATA_FLAG_PROGRAMS` in `scripts/comment_execution.py`; any other program given such a flag is reported. -- Static `sudo`, `env` and `nohup` interpreter wrappers with supported options - are scanned; dynamic or unsupported wrapper options fail closed. Static inner - shell commands passed as one argument to runners such as `ssh`, `watch` or - `xargs` are followed; dynamically assembled commands are not. +- Static `sudo`, `env` and `nohup` interpreter wrappers and `sudo`/`env` + literal-producer wrappers with supported options are scanned; dynamic or + unsupported wrapper options fail closed. Static inner shell commands passed + as one argument to runners such as `ssh`, `watch` or `xargs` are followed; + dynamically assembled commands are not. - HTML comments and MyST `%` lines in Markdown prose are not scanned. In the maintained docs they are copyright headers, generator start and end markers, `` markers read by `scripts/blog_content_validation.py`, diff --git a/scripts/comment_payloads.py b/scripts/comment_payloads.py index b8cd30a2b7..387325e180 100644 --- a/scripts/comment_payloads.py +++ b/scripts/comment_payloads.py @@ -1351,6 +1351,8 @@ def runner_command_start(words: list[str | None], command: str) -> int: PIPED_ENV_VALUE_OPTIONS = RUNNER_VALUE_OPTIONS["env"] - {"-S", "--split-string"} PIPED_ENV_ASSIGNMENT = re.compile(r"[A-Za-z_][A-Za-z0-9_]*=.*") +PIPED_SSH_NO_STDIN_FLAGS = {"-n", "-f", "-N"} + def runner_command_index(words: list[str | None], command: str) -> int: value_options = RUNNER_VALUE_OPTIONS.get(command, set()) @@ -1528,15 +1530,18 @@ def neutral_list_operators(text: str) -> str: def chunk_can_hold_comment(chunk: str) -> bool: - if SHELL_MODELED_INLINE.search(chunk) or ( - not re.search(r"(?= 0 and markers.search(chunk, 0, pipe): + return True return False @@ -1686,11 +1691,110 @@ def piped_redirect_effect(commands: list) -> tuple[bool, bool]: return producer_stdout_redirected, consumer_stdin_redirected +def piped_runner_command_words(words: list) -> list: + if len(words) != 1 or words[0].parts: + return words + try: + trees = bashlex.parse(words[0].word) + except BASHLEX_FAILURES as exc: + raise ValueError("piped runner command requires an adapter") from exc + if len(trees) != 1 or trees[0].kind != "command": + raise ValueError("piped runner command requires an adapter") + command_words = [part for part in trees[0].parts if part.kind == "word"] + if not command_words or len(command_words) != len(trees[0].parts): + raise ValueError("piped runner command requires an adapter") + return command_words + + +def piped_ssh_consumer_words(words: list) -> tuple[list, str]: + static = [None if word.parts else word.word for word in words] + first = runner_command_index(static, "ssh") + options = static[1:first] + if any(word is None for word in options): + raise ValueError("dynamic piped ssh options require an adapter") + if PIPED_SSH_NO_STDIN_FLAGS.intersection(options): + return words, "ssh" + if len(static) - first < 2: + raise ValueError("piped ssh login-shell input requires an adapter") + remote_words = piped_runner_command_words(words[first + 1 :]) + if remote_words[0].parts: + raise ValueError("dynamic piped ssh command requires an adapter") + remote_command = remote_words[0].word.rsplit("/", 1)[-1] + if remote_command in {"env", "sudo", "nohup"}: + return piped_consumer_words(remote_words) + if re.fullmatch(r"python[0-9.]*", remote_command) or remote_command in SHELL_NESTED_TARGETS: + return remote_words, remote_command + return words, "ssh" + + +def piped_watch_consumer_words(words: list) -> tuple[list, str]: + static = [None if word.parts else word.word for word in words] + first = runner_command_index(static, "watch") + if any(word is None for word in static[1:first]): + raise ValueError("dynamic piped watch options require an adapter") + if first >= len(words): + return words, "watch" + target = piped_runner_command_words(words[first:]) + if any(word.parts for word in target): + raise ValueError("dynamic piped watch command requires an adapter") + nested = target[0].word.rsplit("/", 1)[-1] + if not re.fullmatch(r"python[0-9.]*", nested) and nested not in SHELL_NESTED_TARGETS: + return words, "watch" + language = piped_consumer_language(nested) + inline_index = None + if language is not None: + try: + inline_index = inline_source_index([word.word for word in target], language) + except ValueError as exc: + raise ValueError("piped watch input routing requires an adapter") from exc + has_file_operand = any(not word.word.startswith("-") and word.word != "-" for word in target[1:]) + if inline_index is None and not has_file_operand: + raise ValueError("piped watch input routing requires an adapter") + return words, "watch" + + +def piped_xargs_consumer_words(words: list) -> tuple[list, str]: + static = [None if word.parts else word.word for word in words] + first = runner_command_index(static, "xargs") + if any(word is None for word in static[1:first]): + raise ValueError("dynamic piped xargs options require an adapter") + if first >= len(words): + return words, "xargs" + target = piped_runner_command_words(words[first:]) + if any(word.parts for word in target): + raise ValueError("dynamic piped xargs command requires an adapter") + nested = target[0].word.rsplit("/", 1)[-1] + if not re.fullmatch(r"python[0-9.]*", nested) and nested not in SHELL_NESTED_TARGETS: + return words, "xargs" + language = piped_consumer_language(nested) + inline_index = None + if language is not None: + try: + inline_index = inline_source_index([word.word for word in target], language) + except ValueError as exc: + raise ValueError("piped xargs input routing requires an adapter") from exc + has_file_operand = any(not word.word.startswith("-") and word.word != "-" for word in target[1:]) + if inline_index is None and not has_file_operand: + raise ValueError("piped xargs input routing requires an adapter") + return words, "xargs" + + def piped_consumer_words(words: list) -> tuple[list, str]: command = words[0].word.rsplit("/", 1)[-1] + if command == "ssh": + return piped_ssh_consumer_words(words) + if command == "watch": + return piped_watch_consumer_words(words) + if command == "xargs": + return piped_xargs_consumer_words(words) if command in {"sudo", "nohup"}: unwrapped, command = unwrap_runner_command(words, command) - if unwrapped is words and any(word.parts for word in words[1:]): + if unwrapped is words: + dynamic_wrapper = any(word.parts for word in words[1:]) + else: + start = len(words) - len(unwrapped) + dynamic_wrapper = any(word.parts for word in words[1:start]) + if dynamic_wrapper: raise ValueError("dynamic piped interpreter consumer requires an adapter") return unwrapped, command if command == "env" and len(words) > 1: @@ -1725,6 +1829,14 @@ def piped_consumer_words(words: list) -> tuple[list, str]: return words, command +def piped_producer_words(words: list) -> tuple[list, str]: + command = words[0].word.rsplit("/", 1)[-1] + if command == "env": + return piped_consumer_words(words) + unwrapped = unwrap_static_command(words) + return unwrapped, unwrapped[0].word.rsplit("/", 1)[-1] + + def piped_passthrough_cat(command: bashlex.ast.node, words: list) -> bool: if not words or words[0].word.rsplit("/", 1)[-1] != "cat": return False @@ -1751,7 +1863,12 @@ def piped_sides(node: bashlex.ast.node) -> list | None: sides = [[part for part in command.parts if part.kind == "word"] for command in commands] if not sides[0] or sides[0][0].parts: return None - if sides[0][0].word.rsplit("/", 1)[-1] not in PIPED_PRODUCERS: + producer_words, producer = piped_producer_words(sides[0]) + if producer not in PIPED_PRODUCERS: + runner = sides[0][0].word.rsplit("/", 1)[-1] + if (runner in SHELL_WRAPPERS or runner == "env") and any(word.parts for word in sides[0][1:]): + if any(piped_may_execute_interpreter(side) for side in sides[1:]): + raise ValueError("dynamic piped producer requires an adapter") return None for index, side in enumerate(sides[1:], start=1): if not side: @@ -1762,10 +1879,10 @@ def piped_sides(node: bashlex.ast.node) -> list | None: continue consumer_words, consumer = piped_consumer_words(side) if piped_consumer_language(consumer) is not None or consumer in SHELL_UNMODELED: - return [sides[0], consumer_words, commands[: index + 1]] + return [producer_words, consumer_words, commands[: index + 1]] if any(piped_may_execute_interpreter(later) for later in sides[index + 1 :]): raise ValueError("piped interpreter input through an unsupported command requires an adapter") - return [sides[0], consumer_words, commands[: index + 1]] + return [producer_words, consumer_words, commands[: index + 1]] return None @@ -1825,14 +1942,6 @@ def piped_stdin_entries(path: str, source: str, offset: int, unit: str, node: ba return [(line, path + "." + resolved, payload, resolved, f"{symbol}:pipe:{producer}")] -def piped_stdin_payloads(path: str, source: str) -> list[tuple[int, str, str, str, str]]: - try: - trees = bashlex.parse(source) - except BASHLEX_FAILURES: - return [] - return [entry for tree in trees for entry in piped_stdin_entries(path, source, 0, source, tree, "")] - - def shell_command_payloads(path: str, source: str) -> list[tuple[int, str, str, str, str]]: if not SHELL_INLINE_INTERPRETER.search(source): return [] diff --git a/tests/unit/scripts/test_comment_policy.py b/tests/unit/scripts/test_comment_policy.py index 7ebb433c6d..83b47b0d37 100644 --- a/tests/unit/scripts/test_comment_policy.py +++ b/tests/unit/scripts/test_comment_policy.py @@ -1449,7 +1449,16 @@ def test_unknown_split_runner_options_fail_closed(source: str) -> None: @pytest.mark.parametrize( "consumer", - ["$interpreter", "${INTERPRETER}", '"$1"', "cat | $interpreter"], + [ + "$interpreter", + "${INTERPRETER}", + '"$1"', + "cat | $interpreter", + 'ssh "$HOST" python3', + 'ssh host "$COMMAND"', + 'xargs "$OPTIONS" python3', + 'watch "$COMMAND"', + ], ) def test_dynamic_pipeline_consumer_fails_closed(consumer: str) -> None: findings = scan("a.sh", f"echo 'pass' | {consumer}", "bash") @@ -1517,6 +1526,9 @@ def test_piped_printf_escaped_percent_matches_shell_output() -> None: ("echo '# hidden' | cat | python3", "# hidden"), ("echo '# hidden' | cat | cat | python3", "# hidden"), ("echo '# hidden' | python3 | cat", "# hidden"), + ("echo '# hidden' | ssh host python3", "# hidden"), + ("sudo echo '# hidden' | python3", "# hidden"), + ("env FOO=1 echo '# hidden' | python3", "# hidden"), ("echo '# hidden' 1>&1 | bash", "# hidden"), ], ) @@ -1526,6 +1538,39 @@ def test_piped_producer_payloads_reach_stdin_interpreters(source: str, expected: assert all("pipe" in finding.symbol for finding in findings) +def test_ssh_pipeline_skips_option_values_before_remote_command() -> None: + findings = scan("a.sh", "echo '# hidden' | ssh -i /opt/python3 host python3", "bash") + assert [(finding.kind, finding.text) for finding in findings] == [("comment", "# hidden")] + + +def test_ssh_single_string_stdin_interpreter_is_scanned() -> None: + findings = scan("a.sh", "echo '# hidden' | ssh host 'python3 -'", "bash") + assert [(finding.kind, finding.text) for finding in findings] == [("comment", "# hidden")] + + +def test_ssh_pipeline_with_key_path_but_no_remote_command_fails_closed() -> None: + findings = scan("a.sh", "echo '# hidden' | ssh -i /opt/python3 host", "bash") + assert [finding.kind for finding in findings] == ["coverage-error"] + + +def test_ssh_pipeline_without_stdin_does_not_scan_remote_input() -> None: + assert scan("a.sh", "echo '# hidden' | ssh -n host python3", "bash") == [] + + +@pytest.mark.parametrize("runner", ["watch -n 1 python3", "watch 'python3 -'", "xargs 'python3 -'"]) +def test_runner_pipeline_to_stdin_interpreter_fails_closed(runner: str) -> None: + findings = scan("a.sh", f"echo '# hidden' | {runner}", "bash") + assert [finding.kind for finding in findings] == ["coverage-error"] + + +@pytest.mark.parametrize( + "source", + ['sudo "$COMMAND" | python3', 'env COMMAND="$COMMAND" echo "# hidden" | python3'], +) +def test_dynamic_wrapped_piped_producer_fails_closed(source: str) -> None: + assert [finding.kind for finding in scan("a.sh", source, "bash")] == ["coverage-error"] + + def test_piped_interpreter_masking_preserves_trailing_shell_comment() -> None: findings = scan("a.sh", "echo 'pass' | python3 # explanation", "bash") assert [(finding.kind, finding.text) for finding in findings] == [("comment", "# explanation")] @@ -1590,6 +1635,11 @@ def test_piped_non_stdin_programs_stay_data(source: str) -> None: assert scan("a.sh", source, "bash") == [] +def test_piped_xargs_stdin_interpreter_fails_closed() -> None: + findings = scan("a.sh", "echo '# hidden' | xargs -a /dev/null python3 -", "bash") + assert [finding.kind for finding in findings] == ["coverage-error"] + + @pytest.mark.parametrize( ("source", "text"), [ @@ -3408,6 +3458,8 @@ def test_every_astro_expression_is_sent_to_the_typescript_scanner() -> None: ("if ! awk -v s=\"$s\" 'BEGIN { exit !(s >= 1) }'; then\n exit 1\nfi\n", []), ("LEVELS=($(psql -c 'SELECT 1 -- note'))\n", ["coverage-error"]), ("echo x | python3 )\n", ["coverage-error"]), + ("echo '# hidden' | perl )\n", ["coverage-error"]), + ("echo x | perl )\n", []), ("echo x | cat )\n", []), ("echo '| python3'\n", []), ], From 019949d28e23a9dff593f9e2da3b72430cc61ac1 Mon Sep 17 00:00:00 2001 From: Joe Harris Date: Sun, 11 Oct 2026 07:40:35 -0400 Subject: [PATCH 11/17] docs(audit): refresh scanner remediation evidence --- .../audits/remediation-contract-evidence.md | 83 +++++++++++++++++-- 1 file changed, 74 insertions(+), 9 deletions(-) diff --git a/_project/audits/remediation-contract-evidence.md b/_project/audits/remediation-contract-evidence.md index 506f1d434e..5935ffbcfa 100644 --- a/_project/audits/remediation-contract-evidence.md +++ b/_project/audits/remediation-contract-evidence.md @@ -2,8 +2,8 @@ develop_sha: 2eb03f3e67ec8f7f1347738f29696fa39ed5f526 measured_at_sha: dd20aed2d53a87a4c34ae74c73a187cfa9bf3c5e checked_sha: dd20aed2d53a87a4c34ae74c73a187cfa9bf3c5e -replay_sha: 8405f5e3c76ecb679e46079c737083d36494ba57 -replay_scope: PR 2888 mirror, parity, throughput contract tests and full fast suite at replay tree 8405f5e3c76ecb679e46079c737083d36494ba57 +replay_sha: 5c33b6075990429f875b441bc43665f6a1e1a991 +replay_scope: PR 2888 workflow and pipeline/publication/full-fast replay at 50183db50c15b46a1dbd474d3bf7cd297f1c2975; PR 2821 D20-D22 scanner fixes at 5c33b6075990429f875b441bc43665f6a1e1a991 --- # Remediation contract evidence @@ -296,7 +296,7 @@ and checks the emitted commit-status fields. A real nightly dispatch on merged `develop` is still required to verify actual GitHub job outputs, artifacts, and status contexts. -### Verification at the rebased replay tree +### Verification at historical replay tree `8405f5e3c76ecb679e46079c737083d36494ba57` Focused workflow tests passed 105: @@ -316,6 +316,31 @@ passed their expected rejection outcome. Docker CLI is unavailable in this environment; no real Compose CLI config or live registry inspection was performed. +### Current replay at `50183db50c15b46a1dbd474d3bf7cd297f1c2975` + +After the branch rebase, the PR #2888 workflow contract selection passed 105 +tests: + +```bash +uv run -- python -m pytest tests/unit/workflows/test_nightly_t3_workflow.py tests/unit/workflows/test_docker_integration_workflow.py tests/unit/workflows/test_nightly_throughput_uat_contract.py -q +``` + +The pipeline/publication seam replay passed 112 tests: + +```bash +uv run -- python -m pytest tests/unit/scripts/explorer_pipeline/test_pipeline.py tests/unit/scripts/publication/test_transaction.py tests/unit/scripts/publication/test_journal.py -q +``` + +The full fast suite passed 37,273 tests, 26 skipped, 861 warnings, and four +subtests in 297.20 seconds: + +```bash +uv run -- python -m pytest -m fast -q +``` + +Full-project `uv run ruff check .` passed; `uv run ty check` exited 0 with 674 +diagnostics. + ## PR #2821 scanner review findings The nine Oracle findings were reproduced on the pre-fix branch. Each fix keeps @@ -496,12 +521,52 @@ The two affected CI parity tests passed with `uv run -- python -m pytest tests/system/test_ci_lint_parity.py::test_non_lint_merge_gate_guards_have_local_equivalent_or_documented_exemption tests/system/test_ci_lint_parity.py::test_merge_gate_local_equivalents_and_exemptions_are_documented -q -n0`. Ruff check and format checks passed for all three changed Python files. +### Findings D20-D22 + +- D20, piped runner consumers: static SSH options are parsed before the remote + command, and a one-string remote command is decomposed before stdin routing. + `ssh -n` does not scan the producer as remote input. `watch` and `xargs` + pipelines to stdin interpreters fail closed when routing is ambiguous. + Regressions cover SSH option values, single-string remote stdin, disabled SSH + stdin, and direct or single-string watch/xargs consumers in + `test_ssh_pipeline_skips_option_values_before_remote_command`, + `test_ssh_single_string_stdin_interpreter_is_scanned`, + `test_ssh_pipeline_without_stdin_does_not_scan_remote_input`, and + `test_runner_pipeline_to_stdin_interpreter_fails_closed`. The + `test_dynamic_pipeline_consumer_fails_closed` rejects dynamic SSH hosts or + commands, dynamic xargs options, and dynamic watch commands. The + `test_piped_xargs_stdin_interpreter_fails_closed` case rejects an xargs + stdin route with ambiguous input. +- D21, wrapped literal producers: `piped_producer_words` unwraps supported + `sudo` and `env` producer forms before reconstructing `echo`/`printf` output. + `test_piped_producer_payloads_reach_stdin_interpreters` covers both wrappers; + `test_dynamic_wrapped_piped_producer_fails_closed` rejects dynamic forms. +- D22, unparseable piped interpreters: `chunk_can_hold_comment` checks for + comment markers before the pipe when the consumer is unmodeled. The + `echo '# hidden' | perl )` positive and `echo x | perl )` control are in + `test_unparseable_shell_chunks_fail_closed_only_when_they_can_hold_a_comment`. + +The focused D20-D22 regression selection passed 55 tests, with 727 deselected: + +```bash +uv run -- python -m pytest tests/unit/scripts/test_comment_policy.py -k 'piped_producer_payloads_reach_stdin_interpreters or dynamic_pipeline_consumer_fails_closed or ssh_pipeline_skips_option_values_before_remote_command or ssh_single_string_stdin_interpreter_is_scanned or ssh_pipeline_with_key_path_but_no_remote_command_fails_closed or ssh_pipeline_without_stdin_does_not_scan_remote_input or runner_pipeline_to_stdin_interpreter_fails_closed or piped_xargs_stdin_interpreter_fails_closed or dynamic_wrapped_piped_producer_fails_closed or unparseable_shell_chunks_fail_closed_only_when_they_can_hold_a_comment' -q -n0 +``` + + +The scanner unit file and full CI parity file passed 792 tests total (782 and +10). +Ruff check and format checks passed for the changed Python files. + +`uv run ty check` exited 0 with 674 diagnostics. + `make comment-policy-check` passed: 5,779 source files, zero violations, zero enforced failures, and all 19 native syntax tests passed. -The latest timed gate run measured 690.49 seconds wall, 375.00 user, and 61.54 -sys, above the historical 358-second wall / 276-second user baseline. Earlier -runs after env-option support ranged from 321.52 to 483.21 seconds wall and -263.31 to 319.96 user. These whole-gate timings use the immutable base checker -in `scripts/run_comment_policy.py`; they do not isolate candidate scanner cost -or supersede the paired scanner measurements recorded above. +The D20-D22 gate run completed in 287.27 seconds wall; user and system times +were not captured. It passed 5,779 files and 19 native syntax tests. An earlier +timed run measured 690.49 seconds wall, 375.00 user, and 61.54 sys, above the +historical 358-second wall / 276-second user baseline. Earlier runs after +env-option support ranged from 321.52 to 483.21 seconds wall and 263.31 to +319.96 user. These whole-gate timings use the immutable base checker in +`scripts/run_comment_policy.py`; they do not isolate candidate scanner cost or +supersede the paired scanner measurements recorded above. From 86c85b22f446209df039341ec7ec64be6dfcb2f8 Mon Sep 17 00:00:00 2001 From: Joe Harris Date: Sun, 11 Oct 2026 07:58:09 -0400 Subject: [PATCH 12/17] fix(comment-policy): parse nested stdin consumers --- docs/internal/comment-policy.md | 9 +++++---- scripts/comment_payloads.py | 10 ++++++---- tests/unit/scripts/test_comment_policy.py | 8 ++++++++ 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/docs/internal/comment-policy.md b/docs/internal/comment-policy.md index 8aea0e4a46..2d2baf7a25 100644 --- a/docs/internal/comment-policy.md +++ b/docs/internal/comment-policy.md @@ -182,10 +182,11 @@ Known gaps, each a place where a comment can pass unreported: `DATA_FLAG_PROGRAMS` in `scripts/comment_execution.py`; any other program given such a flag is reported. - Static `sudo`, `env` and `nohup` interpreter wrappers and `sudo`/`env` - literal-producer wrappers with supported options are scanned; dynamic or - unsupported wrapper options fail closed. Static inner shell commands passed - as one argument to runners such as `ssh`, `watch` or `xargs` are followed; - dynamically assembled commands are not. + literal-producer wrappers with supported options are scanned, including + nested static wrapper chains; dynamic or unsupported wrapper options fail + closed. Static inner shell commands passed as one argument to runners such as + `ssh`, `watch` or `xargs` are followed; dynamically assembled commands are + not. - HTML comments and MyST `%` lines in Markdown prose are not scanned. In the maintained docs they are copyright headers, generator start and end markers, `` markers read by `scripts/blog_content_validation.py`, diff --git a/scripts/comment_payloads.py b/scripts/comment_payloads.py index 387325e180..bebebe047f 100644 --- a/scripts/comment_payloads.py +++ b/scripts/comment_payloads.py @@ -1796,7 +1796,9 @@ def piped_consumer_words(words: list) -> tuple[list, str]: dynamic_wrapper = any(word.parts for word in words[1:start]) if dynamic_wrapper: raise ValueError("dynamic piped interpreter consumer requires an adapter") - return unwrapped, command + if unwrapped is words: + return unwrapped, command + return piped_consumer_words(unwrapped) if command == "env" and len(words) > 1: index = 1 while index < len(words): @@ -1809,7 +1811,7 @@ def piped_consumer_words(words: list) -> tuple[list, str]: continue if value.startswith("-"): break - return words[index:], value.rsplit("/", 1)[-1] + return piped_consumer_words(words[index:]) static = [None if word.parts else word.word for word in words[1:]] tail = _wrapper_tail( static, @@ -1825,7 +1827,7 @@ def piped_consumer_words(words: list) -> tuple[list, str]: unwrapped = words[consumed + 1 :] if not unwrapped or unwrapped[0].parts: raise ValueError("dynamic piped interpreter consumer requires an adapter") - return unwrapped, unwrapped[0].word.rsplit("/", 1)[-1] + return piped_consumer_words(unwrapped) return words, command @@ -1904,7 +1906,7 @@ def piped_bash_operands(static: list[str]) -> list[str]: for word in static: if skip_operand: skip_operand = False - elif word in {"-o", "+o"}: + elif word in {"-o", "+o", "-O", "+O"}: skip_operand = True else: narrowed.append(word) diff --git a/tests/unit/scripts/test_comment_policy.py b/tests/unit/scripts/test_comment_policy.py index 83b47b0d37..18e652d91b 100644 --- a/tests/unit/scripts/test_comment_policy.py +++ b/tests/unit/scripts/test_comment_policy.py @@ -1529,6 +1529,8 @@ def test_piped_printf_escaped_percent_matches_shell_output() -> None: ("echo '# hidden' | ssh host python3", "# hidden"), ("sudo echo '# hidden' | python3", "# hidden"), ("env FOO=1 echo '# hidden' | python3", "# hidden"), + ("echo '# hidden' | sudo env python3", "# hidden"), + ("echo '# hidden' | env FOO=1 sudo python3", "# hidden"), ("echo '# hidden' 1>&1 | bash", "# hidden"), ], ) @@ -1635,6 +1637,12 @@ def test_piped_non_stdin_programs_stay_data(source: str) -> None: assert scan("a.sh", source, "bash") == [] +@pytest.mark.parametrize("option", ["-O", "+O"]) +def test_piped_bash_shopt_option_is_not_treated_as_script_operand(option: str) -> None: + findings = scan("a.sh", f"echo '# hidden' | bash {option} extglob", "bash") + assert [(finding.kind, finding.text) for finding in findings] == [("comment", "# hidden")] + + def test_piped_xargs_stdin_interpreter_fails_closed() -> None: findings = scan("a.sh", "echo '# hidden' | xargs -a /dev/null python3 -", "bash") assert [finding.kind for finding in findings] == ["coverage-error"] From 1bee4cc6771ce1939c9eee57c0f15b26157ef2ad Mon Sep 17 00:00:00 2001 From: Joe Harris Date: Sun, 11 Oct 2026 08:06:10 -0400 Subject: [PATCH 13/17] docs(audit): record nested pipeline scanner findings --- .../audits/remediation-contract-evidence.md | 27 +++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/_project/audits/remediation-contract-evidence.md b/_project/audits/remediation-contract-evidence.md index 5935ffbcfa..8360a30e47 100644 --- a/_project/audits/remediation-contract-evidence.md +++ b/_project/audits/remediation-contract-evidence.md @@ -2,8 +2,8 @@ develop_sha: 2eb03f3e67ec8f7f1347738f29696fa39ed5f526 measured_at_sha: dd20aed2d53a87a4c34ae74c73a187cfa9bf3c5e checked_sha: dd20aed2d53a87a4c34ae74c73a187cfa9bf3c5e -replay_sha: 5c33b6075990429f875b441bc43665f6a1e1a991 -replay_scope: PR 2888 workflow and pipeline/publication/full-fast replay at 50183db50c15b46a1dbd474d3bf7cd297f1c2975; PR 2821 D20-D22 scanner fixes at 5c33b6075990429f875b441bc43665f6a1e1a991 +replay_sha: 86c85b22f446209df039341ec7ec64be6dfcb2f8 +replay_scope: PR 2888 workflow and pipeline/publication/full-fast replay at 50183db50c15b46a1dbd474d3bf7cd297f1c2975; PR 2821 D20-D24 scanner fixes, focused and full scanner verification at 86c85b22f446209df039341ec7ec64be6dfcb2f8 --- # Remediation contract evidence @@ -570,3 +570,26 @@ env-option support ranged from 321.52 to 483.21 seconds wall and 263.31 to 319.96 user. These whole-gate timings use the immutable base checker in `scripts/run_comment_policy.py`; they do not isolate candidate scanner cost or supersede the paired scanner measurements recorded above. + +### Findings D23-D24 + +- D23, nested piped runner wrappers: `piped_consumer_words` recursively + resolves static wrapper chains until it reaches the effective consumer. + `test_piped_producer_payloads_reach_stdin_interpreters` covers both + `sudo env python3` and `env FOO=1 sudo python3` forms. The latter missed the + stdin comment before the fix; the former already found it. +- D24, Bash option operands: `piped_bash_operands` skips the argument to + `-O` and `+O` before deciding whether Bash has a script-file operand. + `test_piped_bash_shopt_option_is_not_treated_as_script_operand` verifies + both forms. + +The focused D23-D24 regression run passed 32 tests. Before the fix, it +reproduced three failures: the nested `env`/`sudo` chain and both Bash option +forms. The full scanner unit file passed 786 tests; the CI parity file passed +10. Ruff check and format checks passed for the changed Python files. +Full-project type checking with `uv run ty check` exited 0 with 674 diagnostics. + +The full `make comment-policy-check` run passed: 5,779 source files, zero +violations, zero enforced failures, and all 19 native syntax tests passed. +Its runtime overlapped with the commit hook's policy run, so no timing +comparison is recorded for this replay. From 9b974213007f16a5c515befeae45e73aa74b9623 Mon Sep 17 00:00:00 2001 From: Joe Harris Date: Sun, 11 Oct 2026 08:13:37 -0400 Subject: [PATCH 14/17] docs(audit): record scanner review verification --- .../audits/remediation-contract-evidence.md | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/_project/audits/remediation-contract-evidence.md b/_project/audits/remediation-contract-evidence.md index 8360a30e47..1246524378 100644 --- a/_project/audits/remediation-contract-evidence.md +++ b/_project/audits/remediation-contract-evidence.md @@ -583,11 +583,21 @@ supersede the paired scanner measurements recorded above. `test_piped_bash_shopt_option_is_not_treated_as_script_operand` verifies both forms. -The focused D23-D24 regression run passed 32 tests. Before the fix, it -reproduced three failures: the nested `env`/`sudo` chain and both Bash option -forms. The full scanner unit file passed 786 tests; the CI parity file passed -10. Ruff check and format checks passed for the changed Python files. -Full-project type checking with `uv run ty check` exited 0 with 674 diagnostics. +The focused D23-D24 regression run passed 32 tests, with 754 deselected. +Before the fix, it reproduced three failures: the nested `env`/`sudo` chain and +both Bash option forms. + +```bash +uv run -- python -m pytest tests/unit/scripts/test_comment_policy.py -k 'piped_producer_payloads_reach_stdin_interpreters or piped_bash_shopt_option_is_not_treated_as_script_operand' -q -n0 +``` + +The combined scanner unit and CI parity run passed 796 tests (786 and 10): + +```bash +uv run -- python -m pytest tests/unit/scripts/test_comment_policy.py tests/system/test_ci_lint_parity.py -q -n0 +``` + +Ruff check and format checks passed for the changed Python files; `uv run -- python -m py_compile scripts/comment_payloads.py` passed. Full-project type checking with `uv run ty check` exited 0 with 674 diagnostics. The full `make comment-policy-check` run passed: 5,779 source files, zero violations, zero enforced failures, and all 19 native syntax tests passed. From a38f4b561cd9d73397bac197672ae32316ba2b5e Mon Sep 17 00:00:00 2001 From: Joe Harris Date: Sun, 11 Oct 2026 08:34:32 -0400 Subject: [PATCH 15/17] fix(comment-policy): scan piped SQL and MDX exports --- docs/internal/comment-policy.md | 17 +++--- scripts/comment_payloads.py | 73 ++++++++++++++++++++--- tests/unit/scripts/test_comment_policy.py | 50 ++++++++++++++++ 3 files changed, 125 insertions(+), 15 deletions(-) diff --git a/docs/internal/comment-policy.md b/docs/internal/comment-policy.md index 2d2baf7a25..a53314e94a 100644 --- a/docs/internal/comment-policy.md +++ b/docs/internal/comment-policy.md @@ -146,13 +146,16 @@ TypeScript parser; SQL-valued strings and execution-sink arguments as SQL; other formats with Pygments lexers; embedded code with its own language check. Shell heredocs and literal `echo`/`printf` pipelines through plain `cat` stages or supported `sudo`, `env` and `nohup` wrappers into supported stdin -interpreters are scanned. Static `sudo`/`env` producer wrappers are also -scanned. Static SSH pipelines into supported remote stdin -interpreters are also scanned; `watch` and `xargs` pipelines to stdin -interpreters fail closed when input routing is ambiguous. MDX top-level ESM -and JSX block comments outside Markdown code spans and fences are extracted. -Executable paths are matched by basename; consumer output redirects preserve -piped input, while redirected stdin fails closed. +interpreters are scanned. Static `sudo`/`env` producer wrappers and static SSH +pipelines into supported remote stdin interpreters are also scanned. SQL-client +stdin for `psql`, `duckdb` and `sqlite3` is scanned when the command reads from +stdin; explicit query/file inputs and `sqlite3 -no-stdin` are not treated as +piped input. `watch` and `xargs` pipelines to stdin interpreters fail closed +when input routing is ambiguous. MDX top-level ESM imports and exports, +including TypeScript `type`, `interface`, `enum`, `namespace`, `declare` and +`abstract` exports, and JSX block comments outside Markdown code spans and +fences are extracted. Executable paths are matched by basename; consumer +output redirects preserve piped input, while redirected stdin fails closed. Unknown input is a coverage error, never a pass, and under blocking enforcement or in strict mode changed files and completed scopes always reject it. While enforcement is advisory a coverage error is reported and counted but does not diff --git a/scripts/comment_payloads.py b/scripts/comment_payloads.py index bebebe047f..22bc8d0d23 100644 --- a/scripts/comment_payloads.py +++ b/scripts/comment_payloads.py @@ -4,6 +4,7 @@ import re import shlex import textwrap +from collections.abc import Sequence from pathlib import PurePosixPath from typing import Any @@ -325,7 +326,9 @@ def astro_expression_sources(path: str, source: str) -> list[tuple[int, str, str MDX_STATEMENT_START = re.compile(r"(?:import|export)(?=\s|$)") MDX_IMPORT_FROM = re.compile(r"\bfrom\s+['\"][^'\"\n]+['\"]") MDX_IMPORT_BARE = re.compile(r"^import\s+['\"]") -MDX_EXPORT_SHAPE = re.compile(r"^export\s+(?:default|const|let|var|function|class|async|\{|\*)") +MDX_EXPORT_SHAPE = re.compile( + r"^export\s+(?:(?:default|const|let|var|function|class|async|type|interface|enum|namespace|declare|abstract)\b|\{|\*)" +) MDX_STATEMENT_CONTINUE = re.compile(r"(?:[{(\[=,:+*/?&|>-]|\bfrom|\bdefault|\bimport|\bexport)\s*$") @@ -719,7 +722,7 @@ def inline_source_index(words: list[str | None], language: str | None) -> int | } -def sql_client_source_index(name: str, words: list[str | None]) -> int | None: +def sql_client_source_index(name: str, words: Sequence[str | None]) -> int | None: if name == "psql": index = 1 while index < len(words): @@ -784,6 +787,53 @@ def sql_client_source_index(name: str, words: list[str | None]) -> int | None: } +def sql_client_reads_stdin(name: str, words: Sequence[str]) -> bool: + source_index = sql_client_source_index(name, words) + if name == "psql": + if source_index is not None: + return False + index = 1 + while index < len(words): + word = words[index] + if word in {"-f", "--file"}: + if index + 1 >= len(words): + raise ValueError("SQL client input file requires explicit support") + return words[index + 1] == "-" + if word.startswith("--file="): + return word.split("=", 1)[1] == "-" + if word.startswith("-f") and word != "-f": + return word[2:] == "-" + index += 2 if word in PSQL_VALUE_OPTIONS else 1 + return True + if name == "sqlite3": + if "-no-stdin" in words: + return False + if source_index is not None and words[source_index - 1] != "-cmd": + return False + positionals = 0 + index = 1 + while index < len(words): + word = words[index] + if word == "-cmd" or word in SQL_CLIENT_VALUE_OPTIONS: + if index + 1 >= len(words): + raise ValueError("SQL client option requires explicit support") + index += 2 + continue + if word in SQL_CLIENT_FLAGS: + index += 1 + continue + if word.startswith("-"): + raise ValueError("SQL client option requires explicit support") + positionals += 1 + if positionals == 2: + return False + index += 1 + return True + if name == "duckdb": + return source_index is None + raise ValueError("unsupported SQL client stdin source") + + def command_words(words: list[str | None]) -> list[str | None]: if not words or words[0] is None: raise ValueError("dynamic shell command receiving a heredoc") @@ -1557,6 +1607,8 @@ def scan_source_payload(text: str, language: str) -> tuple[str, str]: def piped_consumer_language(command: str) -> str | None: if re.fullmatch(r"python[0-9.]*", command): return "python" + if command in SQL_CLIENTS: + return "sql" return {"node": "javascript", "sh": "bash", "bash": "bash", "zsh": "bash"}.get(command) @@ -1924,18 +1976,23 @@ def piped_stdin_entries(path: str, source: str, offset: int, unit: str, node: ba if consumer_stdin_redirected: raise ValueError("piped interpreter stdin redirection requires an adapter") producer = producer_words[0].word.rsplit("/", 1)[-1] - language = piped_consumer_language(consumer_words[0].word.rsplit("/", 1)[-1]) + consumer = consumer_words[0].word.rsplit("/", 1)[-1] + language = piped_consumer_language(consumer) if language is None: piped_unmodeled_check(consumer_words) return [] static = [word.word for word in consumer_words if not word.parts] if len(static) != len(consumer_words): raise ValueError("dynamic piped interpreter consumer requires an adapter") - if inline_source_index(static, language) is not None: - return [] - operands = piped_bash_operands(static[1:]) if language == "bash" else static[1:] - if any(not word.startswith("-") and word != "-" for word in operands): - return [] + if language == "sql": + if not sql_client_reads_stdin(consumer, static): + return [] + else: + if inline_source_index(static, language) is not None: + return [] + operands = piped_bash_operands(static[1:]) if language == "bash" else static[1:] + if any(not word.startswith("-") and word != "-" for word in operands): + return [] text = piped_producer_text(producer_words) if text is None: return [] diff --git a/tests/unit/scripts/test_comment_policy.py b/tests/unit/scripts/test_comment_policy.py index 18e652d91b..e832b0b286 100644 --- a/tests/unit/scripts/test_comment_policy.py +++ b/tests/unit/scripts/test_comment_policy.py @@ -1605,6 +1605,33 @@ def test_piped_node_source_reports_javascript_comment() -> None: assert [(finding.kind, finding.text) for finding in findings] == [("comment", "// explanation")] +@pytest.mark.parametrize( + "source", + [ + "echo 'SELECT 1 -- hidden' | psql", + "echo 'SELECT 1 -- hidden' | psql --file=-", + "echo 'SELECT 1 -- hidden' | sqlite3 benchmark.db", + "echo 'SELECT 1 -- hidden' | duckdb benchmark.db", + "echo 'SELECT 1 -- hidden' | sqlite3 -cmd 'SELECT 0' benchmark.db", + ], +) +def test_piped_sql_client_stdin_reports_comments(source: str) -> None: + findings = scan_sources(ROOT, {"a.sh": source.encode()}, policy()) + assert [(finding.kind, finding.text) for finding in findings] == [("comment", "-- hidden")] + + +@pytest.mark.parametrize( + "source", + [ + "echo 'SELECT 1 -- hidden' | psql -f queries.sql", + "echo 'SELECT 1 -- hidden' | sqlite3 -no-stdin benchmark.db", + "echo 'SELECT 1 -- hidden' | duckdb benchmark.db 'SELECT 1'", + ], +) +def test_piped_sql_clients_with_other_input_do_not_scan_pipe(source: str) -> None: + assert scan_sources(ROOT, {"a.sh": source.encode()}, policy()) == [] + + @pytest.mark.parametrize( "source", [ @@ -3608,6 +3635,12 @@ def test_mdx_prose_and_fences_are_scanned_once() -> None: [ "import regulations are strict\n", "export controls are important\n", + "export types are important\n", + "export interfaces are important\n", + "export enums are important\n", + "export namespaces are important\n", + "export declares are important\n", + "export abstracts are important\n", "important notes follow\n", ], ) @@ -3642,6 +3675,23 @@ def test_mdx_imports_reach_the_typescript_scanner(source: str) -> None: assert [f.line for f in findings] == [2 if source.startswith("import {\n") else 1] +@pytest.mark.parametrize( + ("source", "comment"), + [ + ("export type Config = {\n value: string; // explanation\n};\n", "// explanation"), + ("export interface Config {\n value: string; // explanation\n}\n", "// explanation"), + ("export enum Config {\n Value, // explanation\n}\n", "// explanation"), + ("export namespace Config {\n export const value = 1; // explanation\n}\n", "// explanation"), + ("export declare const config: string; // explanation\n", "// explanation"), + ("export abstract class Config { // explanation\n}\n", "// explanation"), + ], +) +def test_mdx_type_like_exports_reach_typescript_scanner(source: str, comment: str) -> None: + path = "website/src/content/docs/page.mdx" + findings = scan_sources(ROOT, {path: source.encode()}, policy()) + assert [(finding.kind, finding.text) for finding in findings] == [("comment", comment)] + + @pytest.mark.parametrize( ("source", "comment"), [ From 12acfd365b313374e057d97cb7191f4a98a5dc7d Mon Sep 17 00:00:00 2001 From: Joe Harris Date: Sun, 11 Oct 2026 09:03:27 -0400 Subject: [PATCH 16/17] fix(comment-policy): respect SQL client no-input modes --- scripts/comment_payloads.py | 113 ++++++++++++++-------- tests/unit/scripts/test_comment_policy.py | 3 + 2 files changed, 77 insertions(+), 39 deletions(-) diff --git a/scripts/comment_payloads.py b/scripts/comment_payloads.py index 22bc8d0d23..3aa9b839fe 100644 --- a/scripts/comment_payloads.py +++ b/scripts/comment_payloads.py @@ -761,6 +761,7 @@ def sql_client_source_index(name: str, words: Sequence[str | None]) -> int | Non SQL_CLIENTS = {"psql", "duckdb", "sqlite3"} +SQL_CLIENT_NO_INPUT_OPTIONS = {"-?", "-V", "--help", "--version", "-help", "-version", "-no-stdin"} PSQL_VALUE_OPTIONS = { "-h", "--host", @@ -787,50 +788,84 @@ def sql_client_source_index(name: str, words: Sequence[str | None]) -> int | Non } -def sql_client_reads_stdin(name: str, words: Sequence[str]) -> bool: - source_index = sql_client_source_index(name, words) - if name == "psql": - if source_index is not None: - return False - index = 1 - while index < len(words): - word = words[index] - if word in {"-f", "--file"}: - if index + 1 >= len(words): - raise ValueError("SQL client input file requires explicit support") - return words[index + 1] == "-" - if word.startswith("--file="): - return word.split("=", 1)[1] == "-" - if word.startswith("-f") and word != "-f": - return word[2:] == "-" - index += 2 if word in PSQL_VALUE_OPTIONS else 1 - return True - if name == "sqlite3": - if "-no-stdin" in words: +def sql_client_exit_option(word: str) -> bool: + return word in SQL_CLIENT_NO_INPUT_OPTIONS or word.startswith(("--help=", "--version=")) + + +def psql_reads_stdin(words: Sequence[str]) -> bool: + index = 1 + while index < len(words): + word = words[index] + if sql_client_exit_option(word): return False - if source_index is not None and words[source_index - 1] != "-cmd": + if word in {"-f", "--file"}: + if index + 1 >= len(words): + raise ValueError("SQL client input file requires explicit support") + return words[index + 1] == "-" + if word.startswith("--file="): + return word.split("=", 1)[1] == "-" + if word.startswith("-f") and word != "-f": + return word[2:] == "-" + if word in PSQL_VALUE_OPTIONS: + if index + 1 >= len(words): + raise ValueError("SQL client option requires explicit support") + index += 2 + else: + index += 1 + return True + + +def sqlite3_reads_stdin(words: Sequence[str], source_index: int | None) -> bool: + if source_index is not None and words[source_index - 1] != "-cmd": + return False + positionals = 0 + index = 1 + while index < len(words): + word = words[index] + if sql_client_exit_option(word): return False - positionals = 0 - index = 1 - while index < len(words): - word = words[index] - if word == "-cmd" or word in SQL_CLIENT_VALUE_OPTIONS: - if index + 1 >= len(words): - raise ValueError("SQL client option requires explicit support") - index += 2 - continue - if word in SQL_CLIENT_FLAGS: - index += 1 - continue - if word.startswith("-"): + if word == "-cmd" or word in SQL_CLIENT_VALUE_OPTIONS: + if index + 1 >= len(words): raise ValueError("SQL client option requires explicit support") - positionals += 1 - if positionals == 2: - return False + index += 2 + continue + if word in SQL_CLIENT_FLAGS: index += 1 - return True + continue + if word.startswith("-"): + raise ValueError("SQL client option requires explicit support") + positionals += 1 + if positionals == 2: + return False + index += 1 + return True + + +def duckdb_reads_stdin(words: Sequence[str], source_index: int | None) -> bool: + if source_index is not None: + return False + index = 1 + while index < len(words): + word = words[index] + if word in SQL_CLIENT_VALUE_OPTIONS: + if index + 1 >= len(words): + raise ValueError("SQL client option requires explicit support") + index += 2 + continue + if sql_client_exit_option(word): + return False + index += 1 + return True + + +def sql_client_reads_stdin(name: str, words: Sequence[str]) -> bool: + source_index = sql_client_source_index(name, words) + if name == "psql": + return source_index is None and psql_reads_stdin(words) + if name == "sqlite3": + return sqlite3_reads_stdin(words, source_index) if name == "duckdb": - return source_index is None + return duckdb_reads_stdin(words, source_index) raise ValueError("unsupported SQL client stdin source") diff --git a/tests/unit/scripts/test_comment_policy.py b/tests/unit/scripts/test_comment_policy.py index e832b0b286..55cea46de9 100644 --- a/tests/unit/scripts/test_comment_policy.py +++ b/tests/unit/scripts/test_comment_policy.py @@ -1626,6 +1626,9 @@ def test_piped_sql_client_stdin_reports_comments(source: str) -> None: "echo 'SELECT 1 -- hidden' | psql -f queries.sql", "echo 'SELECT 1 -- hidden' | sqlite3 -no-stdin benchmark.db", "echo 'SELECT 1 -- hidden' | duckdb benchmark.db 'SELECT 1'", + "echo 'SELECT 1 -- hidden' | psql --help", + "echo 'SELECT 1 -- hidden' | sqlite3 -help", + "echo 'SELECT 1 -- hidden' | duckdb -version", ], ) def test_piped_sql_clients_with_other_input_do_not_scan_pipe(source: str) -> None: From e741df1b75275da3c42c83afd990c97e94f5dcbb Mon Sep 17 00:00:00 2001 From: Joe Harris Date: Sun, 11 Oct 2026 09:09:41 -0400 Subject: [PATCH 17/17] docs(audit): record D25-D26 scanner verification --- .../audits/remediation-contract-evidence.md | 45 ++++++++++++++++++- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/_project/audits/remediation-contract-evidence.md b/_project/audits/remediation-contract-evidence.md index 1246524378..91ca951d6e 100644 --- a/_project/audits/remediation-contract-evidence.md +++ b/_project/audits/remediation-contract-evidence.md @@ -2,8 +2,8 @@ develop_sha: 2eb03f3e67ec8f7f1347738f29696fa39ed5f526 measured_at_sha: dd20aed2d53a87a4c34ae74c73a187cfa9bf3c5e checked_sha: dd20aed2d53a87a4c34ae74c73a187cfa9bf3c5e -replay_sha: 86c85b22f446209df039341ec7ec64be6dfcb2f8 -replay_scope: PR 2888 workflow and pipeline/publication/full-fast replay at 50183db50c15b46a1dbd474d3bf7cd297f1c2975; PR 2821 D20-D24 scanner fixes, focused and full scanner verification at 86c85b22f446209df039341ec7ec64be6dfcb2f8 +replay_sha: 12acfd365b313374e057d97cb7191f4a98a5dc7d +replay_scope: PR 2888 workflow and pipeline/publication/full-fast replay at 50183db50c15b46a1dbd474d3bf7cd297f1c2975; PR 2821 D20-D26 scanner fixes, focused and full scanner verification at 12acfd365b313374e057d97cb7191f4a98a5dc7d --- # Remediation contract evidence @@ -603,3 +603,44 @@ The full `make comment-policy-check` run passed: 5,779 source files, zero violations, zero enforced failures, and all 19 native syntax tests passed. Its runtime overlapped with the commit hook's policy run, so no timing comparison is recorded for this replay. + +### Findings D25-D26 + +D25, piped SQL-client stdin: `piped_consumer_language` routes `psql`, +`sqlite3` and `duckdb` to the SQL scanner when their selected input mode reads +stdin. `test_piped_sql_client_stdin_reports_comments` covers five stdin forms: +psql's default input and `--file=-`, SQLite's database and `-cmd` forms, and +DuckDB's database form. Each reports the SQL comment. The companion +`test_piped_sql_clients_with_other_input_do_not_scan_pipe` rejects six +controls: psql file input, SQLite `-no-stdin`, DuckDB positional SQL, and a +help/version mode for each client. + +Producer-to-consumer trace: the shell producer writes the SQL bytes; +`piped_stdin_entries` checks the effective SQL-client input mode and routes +stdin through `scan_source_payload` as SQL; the SQL adapter returns the comment +finding with its source location. + +D26, MDX type-like exports: `MDX_EXPORT_SHAPE` retains `type`, `interface`, +`enum`, `namespace`, `declare` and `abstract` export forms. +`test_mdx_type_like_exports_reach_typescript_scanner` exercises all six forms +and requires each inline comment finding. Nine prose lookalikes, including +keyword-prefix forms, remain clean in +`test_mdx_prose_import_lookalikes_are_not_code`. + +Producer-to-consumer trace: `mdx_js_sources` preserves each complete ESM +declaration and routes it through the TypeScript scanner; the returned finding +maps back to the MDX source line. The regex word boundary rejects keyword-prefix +prose instead of treating it as code. + +The focused regression selection passed 26 tests (5 SQL stdin cases, 6 SQL +input-mode controls, 6 MDX declaration forms and 9 prose controls): + +```bash +uv run -- python -m pytest tests/unit/scripts/test_comment_policy.py -k 'piped_sql_client_stdin_reports_comments or piped_sql_clients_with_other_input_do_not_scan_pipe or mdx_type_like_exports_reach_typescript_scanner or mdx_prose_import_lookalikes_are_not_code' -q -n0 +``` + +The combined scanner unit and CI parity run passed 819 tests. Ruff check and +format passed for both changed Python files. Full-project `uv run ty check` +reported 674 diagnostics with exit status 0. `make comment-policy-check` +passed 5,779 source files with zero violations, zero enforced failures and all +19 native syntax tests passing.