Repository navigation
fix(pe3): treat ignore-list code fences in Markdown as ignore-file entries - #831
jwcastillo wants to merge 2 commits into
Conversation
…tries NVIDIA#787 stopped PE3 from flagging `.env` and other credential paths when they are single-pattern lines of a real ignore file (.gitignore, .dockerignore, ...). The same lines shown in a Markdown code block labelled as an ignore list (```gitignore, ```git-ignore, ```ignore, ```dockerignore) still raise HIGH PE3, which is common in skills that document a project's .gitignore. Fenced blocks whose info string starts with one of those languages now go through the existing `_is_ignore_file_exclusion_entry` check, so only lines that are one pattern token are exempt. Comments, negations (`!.env`), commands, host paths, prose after the fence, other fence languages, and unfenced lines keep their findings. Only Markdown and text files are affected. Fixes NVIDIA#830. Refs NVIDIA#644. Signed-off-by: Wen <jwcastillo@gmail.com>
yashrajp22
left a comment
There was a problem hiding this comment.
One remaining integration issue: the ignore-list exemption is lost across production scan windows. Confirmed in source and a freshly built wheel; the inline comment includes the boundary case.
| reference_material = is_reference_material(file_path, file_type) | ||
| ignore_pattern_file = _is_ignore_pattern_file(file_path) | ||
| ignore_list_fences = ( | ||
| _markdown_ignore_list_fence_ranges(content) if file_type in {"markdown", "text"} else [] |
There was a problem hiding this comment.
Could we preserve the ignore-list language across scan windows? The production runner rebuilds an open fence using only its marker and width, so later windows lose the gitignore label this helper needs. I reproduced this with a 256,001-character Markdown file containing one gitignore block and .env near the end: direct analyze() returns no PE3, but the runner and CLI still report it at line 36,571, in both source and a fresh wheel. The 256,000-character control passes. Please carry the language into the window context and add a regression check through the production runner.
There was a problem hiding this comment.
Thanks @yashrajp22, confirmed and fixed in cdc7599.
Cause: _markdown_context_prefix rebuilt an open fence as marker * width + "\n", so every window after the first saw a bare fence and the gitignore label was gone.
Fix (static_runner.py): the fence state and the open/close transitions now also record the opening info string, and the synthetic opening line includes it (```gitignore). This applies to all three prefix cases: inside a fence, a window starting on the opening line, and a window starting on the closing line. Findings inside the synthetic prefix still map before the owned range, so the prefix itself does not add findings.
Regression checks through run_static_patterns (TestIgnoreListFenceAcrossScanWindows):
- a ```gitignore fence longer than
SECURITY_VIEW_WINDOW_CHARSwith `.env` in a later window: no PE3 (failed before the fix at line 36574); - the same file with a ```bash fence: PE3 still reported on the
.envline.
CLI check on a 256,001-character SKILL.md with one gitignore block and .env at the end: no PE3 on this branch.
…s a fence Long files are scanned in windows. A window that starts inside a fenced block gets a synthetic opening fence, which only carried the marker and width. Analyzers that depend on the fence language, such as the PE3 ignore-list fence exemption, lost it after the first window, so `.env` inside a long ```gitignore block was still reported by the runner and CLI. Fence state and transitions now record the opening info string, and the synthetic opening line includes it. Signed-off-by: Wen <jwcastillo@gmail.com>
Problem
#787 stopped PE3 from flagging single-pattern lines of real ignore files. The same lines shown in a Markdown code block labelled as an ignore list still raise HIGH PE3 (issue #830):
Fix
static_patterns_privilege_escalation.pynow collects the body ranges of fenced blocks whose info string starts with an ignore-list language:gitignore,git-ignore,ignore(GitHub Linguist's "Ignore List" aliases) ordockerignore, case-insensitive, first word only. It does this for Markdown and text files only. A match inside such a block goes through the existing_is_ignore_file_exclusion_entrycheck, exactly like a line of a real ignore file.Still flagged:
!negations, commands, host paths and any line that is not one pattern token;bash,text,yaml, no language,gitignore-like, ...);Tests
tests/nodes/analyzers/test_pe3_ignore_file_entries.pygets a newTestIgnoreListCodeFencesInMarkdownclass with 33 tests:main@ 4c52fbbmake lintandmake formatpass.pytest -m "not integration and not provider" tests/gives 13702 passed, 20 skipped, 4 xfailed and 2 failed. The 2 failures aretest_suppression.py::test_dump_baseline_concurrent_writers_publish_complete_documents[.yaml/.json], which failed under load in the full serial run. Rerun on their own, they pass on both this branch andmain.Real-world check
skillspector scan skills/git-workflow --no-llmon affaan-m/ECC:mainreports PE3 on lines 522 and 523 (.envand.env.localinside the ```gitignore block) and on line 669.git add .env # Contains API keysin a "BAD" example. It is a command, so it should stay flagged.Fixes #830. Refs #644.
Prepared with Claude Code; I reviewed and tested the change.