Fix #596: Strict parsing tests for makefile structural signatures - #721
Merged
Conversation
Part of epic #518. Adds strict positive/negative, ambiguity, nested-delimiter, and ReDoS regression coverage in tests/core_engine/test_language_standards_strict.py for all 39 non-None makefile structural signatures, and fixes five real bugs found while writing that coverage: 1. func_start's special-target exclusion list had .PRECIOUS and .SECONDARY each listed twice (dead redundancy) while missing .ONESHELL, .NOTINTERMEDIATE, and .LOW_RESOLUTION_TIME (the latter two added in GNU Make 4.4, this section's own target_version) -- all three were hallucinated as real func_start matches, e.g. ".ONESHELL:" captured as if it were a target/function name. 2. safety_bypasses only matched the ignore-errors recipe prefix `-` when the command immediately followed with no whitespace. Verified against real GNU Make 4.3 that "-rm ..." and "- rm ..." are both valid, equally common forms (Make strips the modifier and any following whitespace); the dash-then-space form silently never matched. 3. debug_prints only matched echo/printf at true line start, missing the common one-liner recipe form written on the same line as its target (e.g. "check: ; @echo ok"). 4. telemetry ($(info ...)), debug_prints ($(warning ...)), and panics_and_aborts ($(error ...)) used a flat [^)\n]* delimiter that truncated at the first nested `)` on realistic nested-call messages (e.g. "$(error missing dep: $(call check_dep,foo))") -- upgraded to the project's one-level-nesting form. 5. All four Hybrid Domain Sensor rules (serialization_parsing, regex_execution, time_date_logic, ipc_rpc_bridges) anchored with `^\s*` instead of the engine's mandated `^[ \t]*` (Rule 5's named anti-pattern). Since `\s` matches `\n` under re.M, this is a confirmed real O(n^2) ReDoS on a long run of blank lines with no keyword -- ~4x time per doubling measured directly (0.0034s/0.0136s/0.0520s/0.2089s/0.8229s at n=500/1000/2000/4000/8000). Fixed to `^[ \t]*`, confirmed linear (~2x per doubling) after the fix. Verification: - tests/core_engine/test_language_standards_strict.py: 1328 passed. - Full default suite (tests/): 2251 passed, 1 deselected, 1 xfailed. - ruff_audit.py --ci / mypy_audit.py --ci / dead_key_audit.py --ci: clean (the 3 mypy findings in network_risk_sensor.py / full_api_network_map.py are pre-existing on main, unrelated to this change, confirmed by reproducing them with this diff stashed). - golden_crucible run in two purpose-built venvs (zero-dep: `pip install -e .` only; full-precision: + networkx/numpy/pandas/xgboost/tiktoken) against the language-crucible corpus: zero diff in both. Confirmed legitimate, not an empty-corpus artifact -- the corpus has 6 real makefiles, but none of them exercise the specific constructs the fixes touch (grepped for special-target lines, dash-space recipes, semicolon one-liners, and nested $(error/warning/info $(...)) calls: no hits). No fixture regeneration needed since there was nothing to bless. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Part of #518, closes #596.
Adds strict positive/negative, ambiguity-sweep, nested-delimiter, and ReDoS
regression coverage in
tests/core_engine/test_language_standards_strict.pyfor all 39 non-
Nonemakefilestructural signatures, per the checklist in#596. Along the way, found and fixed five real bugs in
makefile'srulesdict in
gitgalaxy/standards/language_standards.py(only that language'ssection was touched).
Signatures covered
All checklist items:
branch,args,structural_boundaries,func_start,safety,safety_bypasses,high_risk_execution,io,api,state_mutation,dead_code,doc,test,concurrency,globals,comprehensions,scientific,reflection_metaprogramming,import(+_dependency_capture),ownership,planned_debt,fragile_debt,spec_exposure,macros,telemetry,debug_prints,panics_and_aborts,thread_sleeps,sync_locks,immutability_locks,cleanup,encapsulation,listeners,test_skip,serialization_parsing,regex_execution,time_date_logic,ipc_rpc_bridges-- 38 signatures viaone parametrized simple-case test, plus a dedicated group-extraction test for
_dependency_capture.Also: comment-style-completeness confirmation (makefile's
line_exclusivefamily has only one comment style,
#, so there's no second style fordead_codeto miss -- documented, not skipped), a lexical-familyno-block-terminator confirmation, and four known-ambiguity checks called out
in the issue (
structural_boundaries/state_mutation,api/cleanuponclean:,func_start/macros,test/regex_execution).Real bugs found and fixed
func_start's special-target exclusion list had dead duplicates and areal gap.
.PRECIOUSand.SECONDARYwere each listed twice in thenegative-lookahead alternation; meanwhile
.ONESHELL,.NOTINTERMEDIATE,and
.LOW_RESOLUTION_TIME(the latter two added in GNU Make 4.4 -- thissection's own
_meta.target_version) were missing entirely. Before thefix,
.ONESHELL:/.NOTINTERMEDIATE:/.LOW_RESOLUTION_TIME:were allhallucinated as real
func_startmatches (e.g. capturing.ONESHELLasif it were a target/function name) -- the same false-positive shape
already documented for C++'s macro spiral.
safety_bypassesmissed the equally-common "dash-space" ignore-errorsform. The pattern required the recipe command to immediately follow the
-modifier with no whitespace. Verified directly against real GNU Make4.3 (
make -f <file>) that\t-rm -f xand\t- rm -f xboth runidentically (Make strips the modifier and any following whitespace before
invoking the shell) -- the dash-then-space form silently never matched.
debug_printsmissed the one-liner recipe form. Anchored only totrue line start, so
check: ; @echo ok(recipe on the same physical lineas its target, separated by
;-- a common shorthand for short recipes)was never detected.
Nested-delimiter truncation (Rule 11) in three
$(...)-messagerules.
telemetry's$(info ...),debug_prints'$(warning ...),and
panics_and_aborts'$(error ...)all used a flat[^)\n]*delimiter, which truncates at the first inner
)on a realistic nestedcall inside the message (e.g.
$(error missing dep: $(call check_dep,foo)), a natural way to build a descriptive messagefrom another macro). Upgraded to the project's established
one-level-nesting bounded form.
Genuine O(n^2) ReDoS across all four Hybrid Domain Sensor rules --
the most severe finding.
serialization_parsing,regex_execution,time_date_logic, andipc_rpc_bridgesall anchored their line-startalternative with
^\s*instead of the engine's mandated^[ \t]*(thisis Rule 5's named anti-pattern, verbatim: "NEVER use
^\s*"). Since\smatches
\nunderre.M,^\s*can cross line boundaries -- on a longrun of blank lines with no keyword anywhere, every blank-line
^position re-scans forward through the rest of the run before failing.
Measured directly before fixing: n=500/1000/2000/4000/8000 blank lines ->
0.0034s/0.0136s/0.0520s/0.2089s/0.8229s, a clean ~4x per doubling (the
textbook O(n^2) signature). After swapping to
^[ \t]*(which cannotcross a newline): 0.00003s..0.00151s across n=500..32000, a clean ~2x per
doubling.
All five are demonstrated old-pattern-reproduces / new-pattern-fixes in
dedicated regression tests, not just asserted.
Verification (all four required gates)
pytest tests/core_engine/test_language_standards_strict.py -q--1328 passed.
pytest tests/(default suite) -- 2251 passed, 1 deselected, 1xfailed. No regressions.
ruff_audit.py --ci/mypy_audit.py --ci/dead_key_audit.py --ci--all clean (no new findings). The 3 pre-existing mypy findings in
network_risk_sensor.py/full_api_network_map.pywere confirmedpresent identically with this diff stashed out, so they're unrelated
environment/dependency-version drift, not a regression from this PR.
ruff format .was run but reformatted ~70 unrelated files under thisvenv's ruff version -- reverted everything except the two files this PR
actually touches, both of which are already
ruff format --check-clean.golden_crucible, run in two purpose-built venvs (zero-dep:pip install -e .only; full-precision: +networkx numpy pandas xgboost tiktoken), against the reallanguage-cruciblecorpus:LANGUAGE_CRUCIBLE_PATH=... pytest -m golden_crucible tests/test_golden_crucible.py -v-- zero diff in both environments.Confirmed this is a legitimate zero-diff, not an empty-corpus artifact:
the corpus has 6 real makefiles (
data/makefile/freebsd/Makefile,data/assembly/bootos/Makefile,data/assembly/hellosilicon/makefile,data/lua/redis/Makefile,data/assembly/cosmopolitan/BUILD.mk, plusone more), but grepping all of them for the specific constructs each fix
touches (special-target lines, dash-space recipes, semicolon one-liner
echo/printf, nested
$(error/warning/info $(...))calls) turned up zerohits -- none of the fixed edge cases happen to appear in this corpus's
makefiles. The hybrid-sensor ReDoS fix is a pure performance/robustness
change and doesn't alter match output on any well-formed real file
either way. Since the diff was genuinely empty and explained, no
golden master fixture regeneration was needed or performed.
Scope
Only
gitgalaxy/standards/language_standards.py(makefile'srulesdictonly) and
tests/core_engine/test_language_standards_strict.py(newmakefile test section, appended at end of file) were touched. No
Nonesignatures were changed. No golden master fixtures were touched (nothing to
regenerate).
🤖 Generated with Claude Code