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 6ffa947b2a..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: 8405f5e3c76ecb679e46079c737083d36494ba57 -replay_scope: PR 2888 mirror, parity, throughput contract tests and full fast suite at replay tree 8405f5e3c76ecb679e46079c737083d36494ba57 +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 @@ -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: @@ -315,3 +315,332 @@ 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. + +### 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 +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`. +- 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. + +### 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. + +### 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`, + `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]` + 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 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 the env-option fix, `make comment-policy-check` passed: 5,779 files, zero +violations, zero enforced failures, and all 19 native syntax tests passed. +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. + +### 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 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. + +### 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, 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. +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. diff --git a/docs/internal/comment-policy.md b/docs/internal/comment-policy.md index dcac7876f2..a53314e94a 100644 --- a/docs/internal/comment-policy.md +++ b/docs/internal/comment-policy.md @@ -144,6 +144,18 @@ 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 through plain `cat` stages +or supported `sudo`, `env` and `nohup` wrappers into supported stdin +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 @@ -172,8 +184,12 @@ 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. +- Static `sudo`, `env` and `nohup` interpreter wrappers and `sudo`/`env` + 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`, @@ -183,10 +199,12 @@ 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 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. +- A program piped to an interpreter on stdin from a producer other than `echo` + or `printf` (such as `curl ... | sh`) is not scanned. +- 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/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_execution.py b/scripts/comment_execution.py index c946edf0d0..54a619684f 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, ) @@ -645,15 +647,23 @@ 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: return args[0], "unsupported", None for index, word in enumerate(words[first:], start=first): nested = word.rsplit("/", 1)[-1] if word else None - if nested and ( - nested.startswith("python") - or nested in {"node", "sh", "bash", "zsh"} | SQL_CLIENTS | UNMODELED_INTERPRETERS + 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( @@ -662,6 +672,18 @@ def runner_payload( return args[0], "unsupported", None 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..3aa9b839fe 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 @@ -321,6 +322,153 @@ def astro_expression_sources(path: str, source: str) -> list[tuple[int, str, str ] +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+['\"]") +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*$") + + +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 + in_block_comment = False + 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 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] == "\\": + closing = line.find(char, closing + 1) + if closing < 0: + 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 + 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 + + +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)) + shape = re.sub(r"/\*.*?\*/", " ", text, flags=re.S) + keep = ( + MDX_IMPORT_FROM.search(shape) or MDX_IMPORT_BARE.match(shape) + if line.startswith("import") + else MDX_EXPORT_SHAPE.match(shape) + ) + if keep: + result.append((index + 1, path + ".tsx", text, "javascript", f"mdx:{len(result)}")) + index = end + else: + index += 1 + 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("\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) + ] + + def example_blocks(source: str) -> list[tuple[int, str, str, str]]: blocks: list[tuple[int, str, str, str]] = [] display_directives = { @@ -574,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): @@ -613,6 +761,7 @@ def sql_client_source_index(name: str, words: list[str | None]) -> int | None: SQL_CLIENTS = {"psql", "duckdb", "sqlite3"} +SQL_CLIENT_NO_INPUT_OPTIONS = {"-?", "-V", "--help", "--version", "-help", "-version", "-no-stdin"} PSQL_VALUE_OPTIONS = { "-h", "--host", @@ -639,6 +788,87 @@ def sql_client_source_index(name: str, words: list[str | None]) -> int | None: } +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 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 + 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 + + +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 duckdb_reads_stdin(words, source_index) + 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") @@ -937,6 +1167,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: @@ -966,11 +1201,22 @@ def unwrap_static_command(words: list) -> list: return words +SHELL_DYNAMIC_PIPED_COMMAND = 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): @@ -1085,10 +1339,185 @@ def runner_command_start(words: list[str | None], command: str) -> int: "nsenter", "flock", "watch", + "sudo", + "nohup", "su", "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(), +} + +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_]*=.*") + +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()) + 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_-]*|--") @@ -1186,16 +1615,427 @@ def neutral_list_operators(text: str) -> str: def chunk_can_hold_comment(chunk: str) -> bool: - if SHELL_MODELED_INLINE.search(chunk): + has_heredoc = bool(re.search(r"(?= 0 and markers.search(chunk, 0, pipe): + return True return False +PIPED_PRODUCERS = {"echo", "printf"} + + +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" + if command in SQL_CLIENTS: + return "sql" + return {"node": "javascript", "sh": "bash", "bash": "bash", "zsh": "bash"}.get(command) + + +def shell_output_escapes(text: str, *, zero_prefix_octal: bool = False) -> 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": + limit = index + (4 if zero_prefix_octal and char == "0" else 3) + end = index + 1 + while end < min(limit, len(text)) and text[end] in "01234567": + end += 1 + 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": + 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_raw = args[0].word + values = [word.word for word in args[1:]] + expanded = [] + value_index = 0 + while True: + index = 0 + while index < len(format_raw): + char = format_raw[index] + if char != "%": + expanded.append(char) + index += 1 + elif format_raw[index : index + 2] == "%%": + expanded.append("%") + index += 2 + 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_raw.replace("%%", "") or value_index >= len(values): + break + format_text, _ = shell_output_escapes("".join(expanded)) + return format_text + + +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:] + 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, zero_prefix_octal=True) + 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:] + 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_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": + continue + descriptor = part.input + if descriptor is None: + descriptor = 0 if part.type in stdin_redirects else 1 + 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 == consumer_index and descriptor == 0: + consumer_stdin_redirected = True + 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: + 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") + 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): + word = words[index] + if word.parts: + break + value = word.word + if value in PIPED_ENV_FLAGS or PIPED_ENV_ASSIGNMENT.fullmatch(value): + index += 1 + continue + if value.startswith("-"): + break + return piped_consumer_words(words[index:]) + 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 piped_consumer_words(unwrapped) + 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 + 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 + 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 not sides[0] or sides[0][0].parts: + return None + 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: + 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 [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 [producer_words, consumer_words, commands[: index + 1]] + return None + + +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", "-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: + pipe_info = piped_sides(node) + if pipe_info is None: + return [] + 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 = producer_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 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 [] + payload, resolved = scan_source_payload(text, language) + line = source[: offset + producer_words[0].pos[0]].count("\n") + 1 + return [(line, path + "." + resolved, payload, resolved, f"{symbol}:pipe:{producer}")] + + 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 +2067,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 +2121,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 +2316,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..c9426b77f2 100644 --- a/scripts/comment_syntax.py +++ b/scripts/comment_syntax.py @@ -4,17 +4,20 @@ 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, shell_payloads, sql_template_sources, @@ -56,6 +59,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 +168,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 +451,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 @@ -595,6 +622,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 +644,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/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 d59b45a789..55cea46de9 100644 --- a/tests/unit/scripts/test_comment_policy.py +++ b/tests/unit/scripts/test_comment_policy.py @@ -1255,6 +1255,46 @@ 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"), + ( + '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: + 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 +1415,309 @@ 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'", + "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: + assert [finding.kind for finding in scan("a.sh", source, "bash")] == ["coverage-error"] + + +@pytest.mark.parametrize( + "consumer", + [ + "$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") + 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")] + + +@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] == [ + ("comment", "# explanation", 'print("%")\n# explanation\n') + ] + + +@pytest.mark.parametrize( + ("source", "expected"), + [ + ("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"), + ("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' | 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"), + ("echo '# hidden' | nohup -- python3", "# hidden"), + ("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' | sudo env python3", "# hidden"), + ("echo '# hidden' | env FOO=1 sudo python3", "# hidden"), + ("echo '# hidden' 1>&1 | bash", "# hidden"), + ], +) +def test_piped_producer_payloads_reach_stdin_interpreters(source: str, expected: str) -> None: + findings = scan("a.sh", source, "bash") + assert [finding.text for finding in findings] == [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")] + + +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")] + + +@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'", + "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: + assert scan_sources(ROOT, {"a.sh": source.encode()}, policy()) == [] + + +@pytest.mark.parametrize( + "source", + [ + '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", + ], +) +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("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"] + + +@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"), + ('ssh host "true # hidden\n/usr/bin/python3"', "# hidden"), + ], +) +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", [ @@ -2782,15 +3125,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 + 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(): - pytest.skip("the TypeScript package for scripts/comment_syntax_js.cjs is not installed") + typescript = resolve_typescript_dir(ROOT) + 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( @@ -2804,6 +3146,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"), [ @@ -3111,8 +3494,12 @@ 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 '# hidden' | perl )\n", ["coverage-error"]), + ("echo x | perl )\n", []), + ("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: @@ -3201,9 +3588,132 @@ 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 */}")]), + ("{ /* 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 */}")]), + ], +) +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] + + +@pytest.mark.parametrize( + ("source", "expected"), + [ + ("`{/* inline */}`", []), + ("`{ /* 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", {}) + 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", + "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", + ], +) +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] + + +@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"), + [ + ("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: + 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") + 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") + ]