diff --git a/.flake8 b/.flake8 new file mode 100644 index 0000000..d41561e --- /dev/null +++ b/.flake8 @@ -0,0 +1,33 @@ +# flake8 configuration for this repository. +# +# Three settings, each with the measurement behind it rather than a preference. +# +# max-line-length = 120 +# Measured before this file was added: the longest line across *.py and +# skills/exe-reverse/scripts/*.py is 110 characters (trace_reduce.py:666). 120 leaves room for a +# rule table or a format string written on one line without forcing a split inside a raw regex, +# which either changes the pattern or produces a concatenation harder to read than the long line. +# A line over 120 is a defect to look at, not a style disagreement. +# +# max-complexity is not set, and C901 is excluded below with the measurement: this repository's +# parsers and dispatchers are long flat if/elif chains over finding kinds and PE structures. Setting +# a threshold above the maximum would be a number chosen to pass rather than a standard; splitting +# them into one function per case would move the shape of the format away from the code that reads +# it, and looking like the specification is this repository's argument. +# +# extend-ignore: +# C901 see above -- reported here as a measurement, not enforced +# E203 slice whitespace (`data[start : end]`); flake8 and every autoformatter disagree, and the +# repository has no autoformatter +# +# Run from the repository root: +# flake8 *.py skills/exe-reverse/scripts/*.py +# +# Enforced in CI: the `lint` job in .github/workflows/ci.yml runs this exact command alongside mypy, +# on a bare interpreter, so the configuration is exercised in the poorest environment it must keep +# working in. + +[flake8] +max-line-length = 120 +extend-ignore = C901,E203 +exclude = .git,__pycache__,tools,build,dist diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..eb8efc8 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,25 @@ +# Normalize line endings so a Windows checkout does not rewrite committed files. +# Scripts stay LF (they are consumed by python/node on POSIX hosts and by the test suite anywhere). +* text=auto eol=lf + +*.py text eol=lf +*.js text eol=lf +*.sh text eol=lf +*.md text eol=lf +*.yml text eol=lf +*.yaml text eol=lf +*.json text eol=lf + +# PowerShell, cmd and batch are the one thing that is happier with CRLF on Windows. +*.ps1 text eol=crlf +*.bat text eol=crlf +*.cmd text eol=crlf + +# Never normalize binary-ish artifacts. +*.png binary +*.exe binary +*.dll binary +*.pdb binary +*.sys binary +*.lib binary +*.zip binary diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 0000000..5de80c4 --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,79 @@ + + +## What this changes + +One paragraph. What changes, and what a reader can do afterwards that they could not before. If the +change is documentation only, say so here and again in the effect surface. + +## Evidence + +Every command below was run in this checkout. Paste the output that decides the question; a summary +of confidence is not a result, and `should pass` is not a run. + +``` +python check_repo.py +python check_refs.py +python check_budget.py +python check_routing.py +python check_commands.py +python -m pytest -q +mypy *.py skills/exe-reverse/scripts/*.py +flake8 *.py skills/exe-reverse/scripts/*.py +``` + +Output that matters: + +``` + +``` + +Claims added or changed: + +| Claim | Grade | What it rests on (command + result) | +|---|---|---| +| | observed / inferred / unverified | | + +- [ ] No claim is graded above the weakest step in its chain. +- [ ] Every `observed` has the command and the output beside it. +- [ ] Every negative result names the range, the pattern and what was searched. A search that + returned nothing is evidence about the search. +- [ ] Every limit cites the measurement or the constraint that set it, rather than a preference. + +## Effect surface + +Check each line that applies. `Documentation only` is a claim, so make it explicitly rather than +leaving the list blank. + +- [ ] `SKILL.md` — the budget gate output is attached above; it is loaded in full on activation. +- [ ] `references/` — a new or renamed file is registered in `references/routing.md`, reachable from + `SKILL.md` or the README, and carries a navigable head if it passes 100 lines. +- [ ] `references/pitfalls.md` — a new entry takes the next number and keeps the same shape. +- [ ] `scripts/` — a new script is in the routing script table, answers `--help` on a bare + interpreter, and its argument table matches every command the docs tell a reader to run. +- [ ] `tests/` — a new script is added to `conftest.SCRIPT_FILES` or declared uncovered with a + reason in `tests/cli/test_cli_coverage.py`; a fixed defect has a test that fails without it. +- [ ] `evidence/` and `docs/tool-verification/` — a capability claim moved in both, and the + measurement record quotes the command it rests on. +- [ ] `README.md` and `README.zh-CN.md` — the two stay in step. +- [ ] `docs/tool-verification/FINDINGS.md` — a defect found by the repository's own tools is + recorded with its symptom, root cause, how it surfaced, and the check that now catches it. +- [ ] Documentation only — no behaviour changed, and the gates above still pass. + +## Readiness declaration (human owner only) + +An agent does not fill this section in. Leave it blank; the reviewer completes it. + +- [ ] The change belongs in this repository rather than in a fork or a downstream project. +- [ ] The boundary of what was not exercised is stated, not implied: every route that needs a + debugger, a driver, a kernel component, a hypervisor, test signing or a second machine is + named as not run here. +- [ ] The serialized evidence is committed, and nothing in it carries target identity or a + machine-specific path. +- [ ] The commit is ready to be fast-forwarded onto `main`. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..155d0fa --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,125 @@ +# CI for the exe-reverse skill repository. +# +# WHAT THIS RUNS +# * the repository's own structural gates: check_repo.py, check_refs.py, check_budget.py, +# check_routing.py, and check_commands.py +# * the pytest suite: tests/unit, tests/cli, tests/integration +# * lint: mypy and flake8 over the gate scripts and the shipped skill scripts +# +# WHAT THIS DOES NOT RUN, ON PURPOSE +# * anything needing a debugger, a driver, a kernel component, a live target process, a hypervisor, +# test signing or a network peer. Those routes are recorded experiments in tests/benchmark.md, +# and what the suite deliberately does not cover is stated in tests/README.md. +# * anything about agent behaviour. Whether a model routes, classifies or refuses correctly is not +# observable from a unit test; skills/exe-reverse/evals/evals.json is the skeleton for that, and +# it has never been executed. +# * the hardware-observation routes. They need particular silicon and privilege; hwprobe.py +# reports what a runner lacks and the suite skips with that reason instead of failing. +# +# The runner is Linux while the skill targets Windows. That is deliberate: the suite observes the +# scripts' behaviour -- parsing, patching, gate logic, exit codes -- and every check that needs a +# Windows fact asks the probe or skips. A green run here is a statement about the *code*, never +# about a route that needs a Windows host. +# +# Coverage is therefore: static structure, documentation anchors, budget, documented commands, +# routing coherence, pure parsers, CLI contracts, fixture construction, typing/style lint and +# no-debugger flows. Nothing more is claimed. +name: ci + +on: + push: + pull_request: +# +# NOTE ON TRIGGERS: no `paths:` filter, deliberately. A filter that watches only `*.py` and the lint +# configuration would skip the structural gates on exactly the changes they exist to catch -- a +# reference index or a documented command edited without touching a script. check_refs.py and +# check_routing.py are documentation gates, so they have to run on documentation commits. + +permissions: + contents: read + +jobs: + test: + name: python ${{ matrix.python-version }} + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python-version: ["3.11", "3.12", "3.13", "3.14"] + + env: + PYTHONIOENCODING: utf-8 + PYTHONDONTWRITEBYTECODE: "1" + # The gates spawn sibling scripts and decode their output. On a Windows runner the default + # codec is the console's (GBK here), and one script's --help text is not GBK-decodable, which + # is reported as a crash in that script rather than as a harness decoding problem. Pinning + # UTF-8 removes the whole class of failure on every platform. + PYTHONUTF8: "1" + + steps: + - uses: actions/checkout@v7 + + - uses: actions/setup-python@v7 + with: + python-version: ${{ matrix.python-version }} + + - name: Install test dependencies + # `tiktoken` is optional for the suite but not for the budget gate's *measurement*: without + # it, check_budget.py falls back to characters/constant, and the two paths must agree. + # Installing it here means CI exercises the real tokenizer path, and the unit tests + # comparing both paths run instead of skipping. + # Not upgrading pip itself: it is not a goal here, and a pip self-upgrade failing would be + # reported as this repository's regression. + run: python -m pip install --upgrade pytest tiktoken + + - name: Structural gate - repository layout + run: python check_repo.py + + - name: Structural gate - documentation anchors + run: python check_refs.py + + - name: Structural gate - context budget + run: python check_budget.py + + - name: Structural gate - documented commands + run: python check_commands.py + + - name: Structural gate - routing inventory matches the entry point + run: python check_routing.py + + - name: Regression suite + run: python -m pytest -q + + - name: Summarise skipped coverage + if: always() + run: python -m pytest -q -ra --no-header -p no:cacheprovider 2>&1 | tail -n 40 + + lint: + # Static checks over the same tracked surface the local commands cover, so what passes locally + # cannot drift from what CI enforces. Both tools read their configuration (mypy.ini, .flake8) + # from the repository root automatically. + # + # The environment is deliberately bare -- only mypy and flake8, none of PyYAML, capstone, + # tiktoken or pyelftools. That is the poorest environment the configuration must keep working + # in, which is what mypy.ini's ignore_missing_imports overrides exist for: CI exercises the + # fallbacks instead of a machine-local install hiding a regression. + # + # One interpreter, not the matrix: the checks are static, and the matrix's job is runtime + # behaviour, which lint does not observe. + name: lint (mypy + flake8) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + + - uses: actions/setup-python@v7 + with: + python-version: "3.12" + + - name: Install lint dependencies + run: python -m pip install --upgrade mypy flake8 + + - name: mypy + run: mypy *.py skills/exe-reverse/scripts/*.py + + - name: flake8 + run: flake8 *.py skills/exe-reverse/scripts/*.py diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..87fb8cb --- /dev/null +++ b/.gitignore @@ -0,0 +1,61 @@ +# Local build products. The kit ships source only: every PE the tests need is generated into a +# pytest temp directory by skills/exe-reverse/scripts/make_fixture_pe.py, so nothing here is an input +# the repository could not rebuild. +*.obj +*.lib +*.exp +*.ilk +*.idb +*.i64 +*.til +*.nam +*.map + +# Working data -- never commit samples, dumps or captures. +*.exe +*.dll +*.sys +*.ocx +*.cpl +*.scr +*.pdb +*.zip +*.7z +*.bin +*.dmp +*.etl +*.pcap +*.png +*.jpg + +# local scratch +_tmp/ +out/ +work/ +audit_out/ + +# local extension-pass workspace: toolchains, vendor SDKs, unpacked installers, scratch notes. +# Nothing under tools/ belongs to the repository -- the skill's own evidence record lives in +# docs/tool-verification/ and carries no target identity or vendored binaries. +tools/ + +# Local copies of the third-party write-ups an absorption pass reads. They are someone else's text, +# not content of this repository: what ships is the absorbed method under an `inferred` label, plus +# the anonymized source line in docs/tool-verification/EXTENSION-absorption-*.md. One `git add -A` +# is all it would take to commit them, so the rule is explicit here rather than resting on "nobody +# has added them yet". +.ref/ + +# sandbox installs made while testing `npx skills add .` against this repo +.claude/ +.agents/ +*.log +*.jsonl + +# python / editors +__pycache__/ +*.pyc +.venv/ +.vscode/ +.idea/ +.DS_Store diff --git a/README.md b/README.md index f3c5f56..84263b9 100644 --- a/README.md +++ b/README.md @@ -162,6 +162,13 @@ Six tools live at the root and are not part of the installed skill. Each carries Consistency has a natural counter-pressure — a broken path fails loudly and someone fixes it. Bloat has none, which is why the budget gate exists. +Those six gates are half of a green run. The other half is `python -m pytest -q` plus the lint pair +(`mypy` and `flake8` over the same surface, configured by `mypy.ini` and `.flake8`). +`.github/workflows/ci.yml` runs all of it on every push — the six gates, the suite on Python +3.11–3.14, and the lint job — and states at the top what it deliberately does not run. A machine +without the optional dependencies is the environment both the lint configuration and the budget +gate's fallback path are required to keep working in. + `tests/benchmark.md` holds the regression matrix (dimension -> public target -> scripts exercised -> measured result -> strength label), and `docs/tool-verification/` is the evidence record behind it: what each script actually did, which independent method confirmed it, and what the target could not diff --git a/README.zh-CN.md b/README.zh-CN.md index ae70810..c26bde6 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -126,6 +126,11 @@ export EXEREV_TOOLS=: # POSIX 一致性有天然的反向压力 —— 坏路径会大声失败，然后有人修。膨胀没有，这就是预算门存在的原因。 +这六道门只是"全绿"的一半，另一半是 `python -m pytest -q` 与 lint 组合（`mypy` 与 `flake8` +跑在同一批文件上，分别由 `mypy.ini` 和 `.flake8` 配置）。`.github/workflows/ci.yml` 在每次 +push 时跑完整套：六道门、Python 3.11–3.14 的测试矩阵、以及 lint 任务，并在文件开头写明它刻意 +不跑什么。缺少可选依赖的机器，正是 lint 配置与预算门回退路径都必须能正常工作的环境。 + `tests/benchmark.md` 保存回归矩阵（维度 -> 公开目标 -> 该行用到的脚本 -> 实测结果 -> 强度标签）， `docs/tool-verification/` 是它背后的证据记录：每个脚本实际做了什么、哪个独立方法确认了它、以及目标 无法演练哪些场景。 diff --git a/check_budget.py b/check_budget.py index f910112..312eecf 100644 --- a/check_budget.py +++ b/check_budget.py @@ -78,9 +78,10 @@ NAV_LINES = 100 # above this, a reference must open navigably CORPUS_FILES_NOTE = 110 # references + scripts, reported as a note past this CORPUS_LINES_NOTE = 26000 # total lines across the same corpus, same treatment # Characters per token, for the no-tiktoken fallback. **Calibrated on SKILL.md against -# tiktoken/cl100k_base** (47,629 chars -> 11,114 tokens = 4.285), so both measurement paths reach -# the same verdict. Getting this wrong made the gate answer differently on CI than locally. -TOKENS_PER_CHAR = 4.285 +# tiktoken/cl100k_base** (26,795 chars -> 6,065 tokens = 4.418, re-measured 2026-10-24), so both +# measurement paths reach the same verdict. Getting this wrong made the gate answer differently on +# CI than locally. +TOKENS_PER_CHAR = 4.418 # A navigable head: one of these must appear in the first NAV_HEAD_LINES lines. NAV_HEAD_LINES = 40 HEDGE_RE = re.compile( @@ -145,9 +146,10 @@ def token_estimate(text): everywhere in this repository, including in this file. When it is not, it falls back to `characters / TOKENS_PER_CHAR`. - The fallback constant is **calibrated against that measurement** rather than guessed: a - markdown-heavy document tokenizes worse per character than the usual /4 rule of thumb, and this - file measured 0.30 characters-to-tokens on SKILL.md (67,011 chars -> 19,929 tokens). + The fallback constant is **calibrated against that measurement** rather than guessed: this + SKILL.md measures 4.418 characters per token (26,795 chars -> 6,065 tokens, cl100k_base). Agreeing + with the "about four" rule of thumb to within a few percent is not close enough to rely on, so + the number is re-derived when the document changes rather than assumed. Why the value matters more than the method: the first version used 3.6 and the gate therefore reached *different verdicts on different machines* -- `WARN over budget` where tiktoken was @@ -198,7 +200,11 @@ def check_index_rows(lines): out = [] for section, key, size in long_rows: hard = size > INDEX_ROW_HARD - out.append(('warn' if hard else 'warn', + # `hard` must actually escalate. This read `'warn' if hard else 'warn'` -- both branches the + # same -- so INDEX_ROW_HARD was defined, computed and never able to fail anything: an index + # row could grow without limit and CI stayed green. Found by injecting an over-long row and + # asking the gate to fail, which is the only way this class of defect shows up. + out.append(('fail' if hard else 'warn', '%s: row for %s is %d chars (soft limit %d)%s' % (section, key, size, INDEX_ROW_SOFT, ' -- that is a paragraph, not an index entry' if hard else ''))) @@ -305,8 +311,11 @@ def check_corpus(): green always-loaded count. """ refs = sorted(glob.glob(os.path.join(REFS, '**', '*.md'), recursive=True)) + # A bytecode cache is not corpus. It is untracked, it appears the first time the suite runs, and + # counting it made this note report 27 scripts where the directory ships 16 -- a measurement that + # changes when a *reader* runs a test is not a measurement of the repository. scripts = sorted(p for p in glob.glob(os.path.join(SKILL_DIR, 'scripts', '**', '*'), recursive=True) - if os.path.isfile(p)) + if os.path.isfile(p) and '__pycache__' not in p.split(os.sep)) def count(paths): total = 0 diff --git a/check_repo.py b/check_repo.py index 880e587..4fdf8ce 100644 --- a/check_repo.py +++ b/check_repo.py @@ -31,7 +31,11 @@ import sys try: import yaml except ImportError: # the skills CLI is the real authority; this is a local gate - yaml = None + # `ignore[assignment]` because the sentinel assignment is only ill-typed where the + # module IS installed (mypy then knows the name as Module); on a bare machine the + # override in mypy.ini makes it Any and the ignore is unused -- warn_unused_ignores + # is off, so both environments stay clean. + yaml = None # type: ignore[assignment] ROOT = os.path.dirname(os.path.abspath(__file__)) SKILLS_DIR = os.path.join(ROOT, 'skills') diff --git a/docs/tool-verification/FINDINGS.md b/docs/tool-verification/FINDINGS.md index a4c0579..6f01570 100644 --- a/docs/tool-verification/FINDINGS.md +++ b/docs/tool-verification/FINDINGS.md @@ -154,9 +154,85 @@ on this host rather than an error. *Fix and check:* long values wrap onto continuation lines instead of being cut. A test asserts a long value survives into the output intact. +## Gate, harness and lint defects + +**F11. The hard index-row limit was computed and then unable to fail anything.** + +*Symptom:* none. The gate reported `ok` on an index row of 450 characters, past the 400-character +limit it declares, and CI stayed green. + +*Root cause:* the finding was appended as `('warn' if hard else 'warn', ...)` -- both branches the +same expression. `INDEX_ROW_HARD` was defined, the comparison was performed, and the level was then +thrown away. The message text even said "that is a paragraph, not an index entry", which is what made +it read as enforcement. + +*How it surfaced:* by injecting an over-long row into a scratch copy and requiring the gate to exit +non-zero. It exited 0. A gate that has only ever been seen passing is not evidence that it can see +anything, and this class of defect is invisible to every test that runs the gate on a conforming tree. + +*Fix and check:* the escalated level is `fail`. `tests/unit/test_budget_measurement.py` asserts that a +row past `INDEX_ROW_HARD` produces `fail` and a row past `INDEX_ROW_SOFT` produces `warn`, so the two +thresholds cannot silently collapse into each other again. + +**F12. The corpus measurement counted a bytecode cache, so its own number changed when a reader ran +the suite.** + +*Symptom:* the budget gate reported `36 references + 27 scripts = 63 files` on a tree that ships 16 +scripts. The count moved between runs on the same commit. + +*Root cause:* the script half of the corpus was `glob(scripts/**/*)` filtered by `isfile`, which +includes `scripts/__pycache__/*.pyc`. Reading a directory is a measurement of the repository; reading +a directory that the reader has been scribbling in is a measurement of the reader. + +*How it surfaced:* comparing the number the gate printed against the number of files the routing gate +said it covers. Two gates measuring the same tree disagreed, and that disagreement was the finding. + +*Fix and check:* paths under `__pycache__` are excluded from the corpus. The count now matches the +routing inventory, and re-running the suite no longer changes it. + +**F13. A test asserted a capability it never checked, so a host without it failed instead of skipping.** + +*Symptom:* `test_prober_can_dump_raw_cpuid_registers` failed on a non-x64 host with "a raw dump was +requested but nothing was reported", while the probe's own report said CPUID was unavailable there. + +*Root cause:* the test asserted that the dump was non-empty without asking whether the host could +produce one. The script was right and the test encoded a Windows assumption. It also contradicted the +suite's own rule that a missing prerequisite is a skip with a printed reason. + +*Fix and check:* the test reads the probe's `cpu` row first and skips with that row's note when the +state is not `available`; the non-empty assertion still runs where CPUID is reachable. The skip text +names the reason, so a green run on Linux is not mistaken for a green run on Windows. + +**F14. A configuration comment pointed at a document that does not exist.** + +*Symptom:* `pytest.ini` sent a reader to `docs/tool-verification/EXTENSION-test-harness.md` for the +suite's coverage policy. No such file has ever been in this repository. + +*Root cause:* the file was inherited from the Android-side repository along with the harness, and the +structural gates check the *skill* tree's references, not a comment in an ini file. A stale path in a +comment fails nothing, which is exactly why it survives. + +*Fix and check:* the comment points at `tests/README.md`, which carries a "what it deliberately does +not do" section. Recorded here rather than silently repaired because the class -- a reference in a +non-markdown file that no gate walks -- is still open. + +**F15. The type checker rejected a sentinel assignment the moment its optional dependency was installed.** + +*Symptom:* `mypy *.py skills/exe-reverse/scripts/*.py` reported `check_repo.py:34: Incompatible types +in assignment (expression has type "None", variable has type Module)` on a machine with PyYAML, and +no error at all on a machine without it. + +*Root cause:* `yaml = None` in the `except ImportError` branch is ill-typed only where the import +succeeded, which is the environment the lint job runs in. The repository had no lint job at the time, +so the two environments never had to agree. + +*Fix and check:* the assignment carries `# type: ignore[assignment]`, with `warn_unused_ignores` off +(already the configuration) so the bare-machine environment stays clean too. `mypy.ini` and `.flake8` +are now enforced by the `lint` job in CI. + ## What this list is for -Ten defects, all in code written during this pass, all of which produced output that looked right. The +Fifteen defects, all in code written or inherited during this pass, all of which produced output that looked right. The pattern is consistent enough to state as a rule: **A check that cannot fail is not a check, and a table written from recall is a table that is wrong in diff --git a/pytest.ini b/pytest.ini index ae3f3c7..35a30df 100644 --- a/pytest.ini +++ b/pytest.ini @@ -6,8 +6,8 @@ # no live target process. # What does *not* live here: anything that needs a running Windows target or a # kernel-mode component. Those routes stay in tests/benchmark.md as recorded -# experiments, and their coverage status is stated in -# docs/tool-verification/EXTENSION-test-harness.md. +# experiments, and what the suite deliberately does not cover is stated in +# tests/README.md (`## What it deliberately does not do`). # # If pytest is not installed: # python -m pip install pytest diff --git a/skills/exe-reverse/SKILL.md b/skills/exe-reverse/SKILL.md index be647ff..7bd8fe5 100644 --- a/skills/exe-reverse/SKILL.md +++ b/skills/exe-reverse/SKILL.md @@ -162,6 +162,8 @@ Load the named file **before your next command**, not after your next three atte | You are about to publish notes, a script or a screenshot of the work | `references/desensitization.md` | | The task has run long and you are unsure what is already proven | `references/long-task-discipline.md` | | A documented command does not run, or a tool "is missing" | `scripts/hwprobe.py` | +| A fix removed the error, and the requester still sees the symptom | `references/pitfalls.md` | +| A verification passed, but you cannot say what would have failed without the change | `references/pitfalls.md` | | You need to know what this skill does not cover before promising anything | `references/coverage-and-limits.md` | ## Start here: classify the target in thirteen questions diff --git a/skills/exe-reverse/references/pitfalls.md b/skills/exe-reverse/references/pitfalls.md index f020e84..b9ad606 100644 --- a/skills/exe-reverse/references/pitfalls.md +++ b/skills/exe-reverse/references/pitfalls.md @@ -12,6 +12,11 @@ That is the shape worth fearing. A crash announces itself; these do not. Each entry states the failure, the mechanism that makes it invisible, and the check that catches it. +P1-P25 are format and mechanism failures of Windows binaries; P26-P34 are failures of the *analysis* +itself — sampling read as observation, a self-built parser trusted, a verification that could not have +failed. Those are written with Windows examples because that is where they were paid for, but the +mechanism does not depend on the platform: read them on a managed assembly and a driver the same way. + --- **P1. A length change, made by someone who thought of it as "just adding a byte".** @@ -242,3 +247,104 @@ once, it cannot be withdrawn from a history. *Check:* scan before committing, and be as careful about over-redacting as about under-redacting: removing tool names and format names destroys the reusable part and removes none of the identity. + +**P26. The error disappeared and the symptom did not.** + +The log line is gone, the dialog that used to appear is gone, and the report says fixed. The +requester then shows the same problem still on screen. The internal signal and the user-visible +outcome are different claims: suppressing one error path proves that path ran, not that the symptom +had only one producer. + +*Check:* define the result as the observable the requester described, reproduce it before the change +so the "after" has something to compare against, and treat a quiet log as absence of evidence rather +than evidence of success. + +**P27. "I did not observe it" treated as "it is not there".** + +A breakpoint that never fires, an ETW provider that logs nothing, a screenshot loop that never +catches the dialog. Sampling is not observation: a transient state can fall entirely between samples, +a provider can be filtering the very event you want, and a breakpoint on a page that is already warm +misses the access. The captures are real and consistently empty, which reads as corroboration when it +is one blind spot repeated. + +*Check:* name what the instrument can and cannot see before drawing a negative conclusion, and ask the +same question with an instrument of a different kind -- a hardware breakpoint instead of a software +one, a file-level search instead of a runtime one, a second provider. + +**P28. Waiting for something only a person can advance.** + +An automation loop polls for a state that will never arrive on its own: an installer's UAC prompt, a +licence dialog waiting for input, a captcha, a "click to continue" in a trial build. Polling looks +productive -- timestamps advance, the screen is unchanged -- and can run for a long time without the +possibility of progress. + +*Check:* before waiting, say what would cause the state to change. If the answer is "a person", drive +the prompt programmatically or hand it back. Bound every wait by change, not by elapsed time. + +**P29. "The search returned nothing" treated as "the bytes are not there".** + +A scan for a known-present string returns zero hits and the route is written off. Usually the needle +was encoded differently from the haystack: UTF-16LE inside a resource or a wide-char table, a string +split across fragments, a compressed resource section, or text that exists only after decryption at +run time. + +*Check:* search more than one encoding before concluding absence (`utf-8` and `utf-16-le` cover most +storage), search the shortest distinctive fragment, look inside containers before declaring the data +missing, and state the negative result with its scope -- "no UTF-8 or UTF-16LE literal in this file" +is a finding; "the string does not exist" is a guess. + +**P30. Your own analysis script's defect, recorded as a target finding.** + +A purpose-written parser prints a plausible table, count or verdict, and it is wrong. It ran clean and +fast, so it was trusted. Common shapes: an operator-precedence mistake that silently widens a mask, a +local helper whose name shadows a stdlib module inside a third-party package, a "precision" number +computed against an incomplete reference, an offset table built with the wrong RVA-to-file mapping. + +*Check:* sanity-check the shape before the content -- a distribution that is implausible for the +format is a bug signal. Cross-check against an independently produced artifact, and exercise any +fallback path against a case whose answer you already know. + +**P31. A verification that could not have failed.** + +A check "passes" while having tested nothing, because the observable is identical whether or not the +change took effect. The patch removes a dialog that also does not appear in the state you tested in, +or the field you edited is not the field the UI reads. + +*Check:* ask, before running it, **what would this look like if the change were absent?** If the +answer is the same, the measurement is void: record it as *not applicable under this condition*, never +as *passed*. + +**P32. One observer's failure silently disarming the others.** + +Several instruments are installed -- hooks in different modules, a debugger plus a logger, two ETW +sessions -- and one of them faults. On Windows the failure is often swallowed: a vectored exception +handler consumes the fault, a hook chain returns early, a thread dies while the process keeps running. +Everything installed after the failure stops reporting, and the absence is read as "nothing happened +there". + +*Check:* after installing N observers, prove each one still fires on a known event. Install them one +at a time when the first fault is unexplained, and treat a sudden drop to zero events as a broken +instrument until proven otherwise. + +**P33. Two tools disagree about the same artifact, and the disagreement is your parser.** + +A disassembler, a PE viewer and a debugger report different entry points, different imports or +different section sizes. The reflex is to distrust the target. In practice the usual cause is a +difference in what each tool means: RVA versus file offset, virtual versus raw size, the section that +*contains* an address versus the section that *starts* at it, or a cached parse of an older copy of +the file. + +*Check:* convert both readings into the same coordinate system (RVA, file offset, module base) before +comparing, and verify one value by hand from the headers. A disagreement you cannot restate in one +coordinate system is not yet a finding. + +**P34. The shell mangled the command before the target ever saw it.** + +A path with spaces, a quote inside an argument, a `--flag=value` that a wrapper re-splits: the tool +receives something other than what was typed and reports a usage error, a missing file, or -- worse -- +succeeds against a different target. PowerShell adds its own layer (argument re-quoting, `--%`, and +`Start-Process -ArgumentList` joining a string differently from an array), and `cmd /c` adds another. + +*Check:* when an argument-heavy command fails in a way that does not match its own parser, reproduce +it with the argument written to a file or passed as an array, and print the argv the child actually +received before changing the command. diff --git a/skills/exe-reverse/references/routing.md b/skills/exe-reverse/references/routing.md index 1f381d1..86f5084 100644 --- a/skills/exe-reverse/references/routing.md +++ b/skills/exe-reverse/references/routing.md @@ -52,6 +52,8 @@ without a check. | You are about to publish notes, a script or a screenshot of the work | `references/desensitization.md` | | The task has run long and you are unsure what is already proven | `references/long-task-discipline.md` | | A documented command does not run, or a tool "is missing" | `scripts/hwprobe.py` | +| A fix removed the error, and the requester still sees the symptom | `references/pitfalls.md` | +| A verification passed, but you cannot say what would have failed without the change | `references/pitfalls.md` | | You need to know what this skill does not cover before promising anything | `references/coverage-and-limits.md` | ## Reference index diff --git a/tests/README.md b/tests/README.md index 74c72cf..b051fbe 100644 --- a/tests/README.md +++ b/tests/README.md @@ -20,9 +20,11 @@ and "the script was measured doing X on this host". | `unit/test_scan_leaks.py` | Every rule firing on a shaped sample, every exemption class staying silent, the three measured deviations from the exemption list, scan position reporting, and the file walk's skips. | | `unit/test_capabilities_registry.py` | The capability table: every atom's kind, basis and probe name, the transitive closure against an independent walk, the dependency graph's acyclicity, `resolve` on missing atoms and on `file:` atoms that are and are not on disk, the declaration parser including the documentation template it must reject, and the `declared` / `mapped` / `unknown` classification. | | `unit/test_doctor.py` | The static dependency pass (stdlib, siblings and a third-party name separated without executing the file), `discover_scripts` scanning rather than listing, the per-script table with an unclassified script still counted, and the environment facts with their keys and severity labels. | +| `unit/test_budget_measurement.py` | The budget gate's two measurement paths agree (real tokenizer versus the calibrated fallback), the fallback constant still matches a fresh measurement of `SKILL.md`, and a row past `INDEX_ROW_HARD` fails rather than warns. | | `unit/test_dart_snapshot_strings.py` | The ELF reader against a generated container, both literal encodings recovered at the offsets the fixture placed them at, the length and limit gates, and the pool counts -- zero on an image carrying neither idiom and non-zero on hand-built bytes carrying each one. | | `cli/test_cli_surface.py` | Help and usage exit codes for every script, the documented exit codes for each script's error paths, and the negative case that inspect-only runs write nothing into the working directory. | -| `cli/test_cli_json.py` | Each script's machine-readable output: keys, values, and the conventions that are not what a reader would assume. | +| `cli/test_cli_coverage.py` | Coverage derived from the scripts directory rather than a hand-written list: every shipped `.py` is either exercised or declared uncovered with a reason, and every one answers `--help` with its usage block. A new script fails here until it is registered. | +| `cli/test_cli_json.py` | Each script's machine-readable output: keys, values, and the conventions that are not what a reader would assume -- including `pe_triage.py`'s identity block on a generated image (and that it writes nothing) and `pe_diff.py`'s self-comparison, which is the control for every other diff. | | `cli/test_cli_registry_surface.py` | The registry and doctor subprocess contracts: `--list` / `--json` / single-capability exit codes and `RESULT=` tokens, the doctor's three blocks and its 0-versus-3 verdict, and the view flags that print one block only. | | `cli/test_cli_dart_snapshot.py` | The snapshot reader's CLI: the refusals and their exit codes, `--find` case-insensitivity and the exit code a miss produces, `--at` by file offset and by virtual address, and the `--json` document. | | `integration/test_pipeline_end_to_end.py` | Build, locate, patch, re-read: the patched bytes, the untouched control site, the recomputed checksum, and a control run in which nothing changes. | diff --git a/tests/cli/test_cli_coverage.py b/tests/cli/test_cli_coverage.py new file mode 100644 index 0000000..953b0db --- /dev/null +++ b/tests/cli/test_cli_coverage.py @@ -0,0 +1,80 @@ +"""Coverage of the shipped scripts, derived from the directory rather than a hand-written list. + +Borrowed mechanism: the sibling apk-reverse repository derives its CLI contract cases from +skills/*/scripts/*.py and from each script's own --help output, so a new script is covered the day +it lands instead of the day someone remembers to add it. What is *not* borrowed is that repository's +single 0-4 exit-code table: this kit publishes a per-script vocabulary (pepatch.py uses 3 for "not a +parseable PE", where the table means "the environment lacks a capability"), and a test that rewrote +those meanings would assert a policy this repository deliberately does not have. + +The guard here is therefore narrower, and it is about *coverage*, not exit codes: + + * every shipped .py is either in conftest.SCRIPT_FILES (some layer of this suite exercises it) or + named in DECLARED_UNCOVERED together with the reason it is not; + * every declared gap carries a real sentence, because "TODO" is how a gap becomes permanent; + * every shipped script -- covered or declared -- answers --help with its usage block, which is the + cheapest possible exercise of its argparse table and its import path. + +A new script fails this file until one of those lists changes. That is the point: the alternative +is a suite that looks complete while silently ignoring half the kit. +""" +import subprocess +import sys + +import pytest + +from conftest import SCRIPTS_DIR, SCRIPT_FILES + +pytestmark = pytest.mark.cli + +# Scripts no layer exercises yet. Each entry is a recorded gap, not an exemption: the reason says +# what would be needed to close it, so the next pass can decide rather than rediscover. +DECLARED_UNCOVERED = { + "plan.py": + "composes capabilities.py and doctor.py into a route from a stated intent. Both halves have " + "their own tests; the composition reads live host facts, so a fixture that pinned its output " + "would pin this machine's capability set instead of the mapping. Closing it needs a stubbed " + "probe, and that stub does not exist yet.", + "preflight.py": + "answers session readiness from live host facts (free space, route prerequisites) plus one " + "sample. The sample half could be pinned to a generated PE; the host half has no generator, " + "and asserting on the runner's environment would encode this machine as an expectation.", +} + + +def shipped_scripts(): + """Every runnable python script the skill ships, sorted.""" + return sorted(p.name for p in SCRIPTS_DIR.glob("*.py")) + + +def test_every_shipped_script_is_exercised_or_declared(): + """A script nobody tests must at least be a script somebody decided not to test.""" + shipped = set(shipped_scripts()) + assert shipped, "no scripts found under %s" % SCRIPTS_DIR + silently_uncovered = shipped - set(SCRIPT_FILES) - set(DECLARED_UNCOVERED) + assert not silently_uncovered, ( + "these scripts are shipped but neither exercised nor declared uncovered: %s. Add a test and " + "register the script in conftest.SCRIPT_FILES, or add it to DECLARED_UNCOVERED with the " + "reason." % sorted(silently_uncovered)) + + +def test_declared_entries_are_neither_stale_nor_hollow(): + """A gap that was closed must leave the list; a gap with no reason is a hidden one.""" + both = set(SCRIPT_FILES) & set(DECLARED_UNCOVERED) + assert not both, ("these scripts are in SCRIPT_FILES and also declared uncovered: %s -- the " + "declaration is stale" % sorted(both)) + for name, reason in DECLARED_UNCOVERED.items(): + assert (SCRIPTS_DIR / name).is_file(), "%s is declared uncovered but does not exist" % name + assert len(reason.split()) >= 12, ( + "%s: the reason must say what would close the gap, not just mark it" % name) + + +@pytest.mark.parametrize("script", shipped_scripts()) +def test_every_shipped_script_answers_help_with_usage(script): + """The usage block is the script's published contract; it must render on every host.""" + proc = subprocess.run([sys.executable, str(SCRIPTS_DIR / script), "--help"], + capture_output=True, text=True, timeout=120, + encoding="utf-8", errors="replace") + assert proc.returncode == 0, "%s --help exited %d: %s" % (script, proc.returncode, proc.stderr) + assert "usage:" in proc.stdout.lower(), "%s printed no usage block" % script + assert script in proc.stdout, "%s does not name itself in its usage line" % script diff --git a/tests/cli/test_cli_json.py b/tests/cli/test_cli_json.py index 1f2dddd..0c94061 100644 --- a/tests/cli/test_cli_json.py +++ b/tests/cli/test_cli_json.py @@ -390,8 +390,48 @@ def test_prober_rows_carry_a_measurement_and_a_note(hwprobe_report): assert row["state"] in ROUTE_STATES or row["state"].startswith("needs-"), row -def test_prober_can_dump_raw_cpuid_registers(run_script): +def test_prober_can_dump_raw_cpuid_registers(run_script, hwprobe_report): + """A requested CPUID dump must be non-empty -- where CPUID is reachable at all. + + The probe's own report says which of those two situations this host is in, so the test reads it + instead of assuming x64 Windows: on a non-x64 or non-Windows host the stub cannot run, and + "nothing was reported" is the correct answer rather than a defect. A missing capability is a + skip with the probe's own note, never a red test (tests/README.md, conftest.py). + """ + cpu = hwprobe_report["cpu"][0] + if cpu["state"] != "available": + pytest.skip("CPUID is not reachable on this host, so there is no register dump to check: %s" + % cpu["note"]) result = run_script("hwprobe.py", "--raw", "--json", timeout=120) assert result.returncode == 0, result.stderr payload = json.loads(result.stdout) assert payload["raw_cpuid"], "a raw dump was requested but nothing was reported" + + +# --- pe_triage --------------------------------------------------------------- + +def test_triage_reports_identity_and_leaves_the_sample_untouched(fixtures, run_script): + """The first script a task should run: it parses, classifies, and must not write.""" + sample = fixtures.pe64 + before = sample.data + result = run_script("pe_triage.py", sample.path, "--json") + assert result.returncode == 0, result.stderr + payload = json.loads(result.stdout) + assert payload["sha256"] == sample.sha256 + assert payload["identity"]["arch"] == "amd64" + assert payload["identity"]["bitness"] == 64 + assert payload["identity"]["kind"] == "exe" + assert sample.data == before, "triage modified the image it was reading" + + +# --- pe_diff ----------------------------------------------------------------- + +def test_diff_of_the_same_build_reports_no_material_change(fixtures, run_script): + """Self-comparison is the control for every other diff: it must find nothing.""" + sample = fixtures.pe64 + result = run_script("pe_diff.py", sample.path, sample.path, "--json") + assert result.returncode == 0, result.stderr + payload = json.loads(result.stdout) + assert payload["verdict"] == "identical-modulo-normalised-fields" + assert payload["material_change"] is False + assert payload["byte_diff"]["differ_bytes"] == 0 diff --git a/tests/conftest.py b/tests/conftest.py index 8468fa4..6af9cc7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -50,6 +50,11 @@ SCRIPT_FILES = ( "dart_snapshot_strings.py", "fold.py", "trace_reduce.py", + # The two route helpers: the first script a task should run on an unknown image, and the + # two-build comparison. Added with tests/cli/test_cli_coverage.py, which fails if a shipped + # script is neither in this tuple nor declared uncovered with a reason. + "pe_triage.py", + "pe_diff.py", ) # A hang detector, not a performance assertion. Nothing in this suite is meant to be slow. diff --git a/tests/unit/test_budget_measurement.py b/tests/unit/test_budget_measurement.py new file mode 100644 index 0000000..8ef1189 --- /dev/null +++ b/tests/unit/test_budget_measurement.py @@ -0,0 +1,137 @@ +"""The budget gate must reach the same verdict whether or not `tiktoken` is installed. + +This is a small test for a defect that cost two red CI runs upstream. `check_budget.py` measures the +always-loaded body in tokens, preferring `tiktoken` and falling back to `characters / N` when it is +absent. The first version's `N` was guessed (3.6) instead of calibrated, so a machine with tiktoken +saw ~10,900 tokens and passed while a machine without it saw ~13,200 and warned -- and the gate +runner (CI's oldest job, and any contributor's box) is the one without it. + +A gate whose answer depends on which packages the runner happens to have installed is not a gate, so +both paths are forced here and compared. + +The third test guards the other half of the same failure: a threshold that is computed and then +unable to fail anything. +""" +from __future__ import annotations + +import importlib +import sys + +import pytest + +from conftest import REPO_ROOT + +pytestmark = pytest.mark.unit + +SKILL_MD = REPO_ROOT / "skills" / "exe-reverse" / "SKILL.md" + + +@pytest.fixture +def budget(): + sys.path.insert(0, str(REPO_ROOT)) + return importlib.import_module("check_budget") + + +def _has_tiktoken(): + """Whether a real tokenizer is available, reported rather than assumed. + + The comparison needs *two* implementations; where there is only one, the honest outcome is a + skip with the install command, not a failure and not a silent pass. CI installs it, so the + property is verified where it matters; a contributor without it has both gates agreeing anyway, + because the fallback is calibrated to the same constant. + """ + try: + import tiktoken # noqa: F401 + return True + except Exception: + return False + + +def test_the_fallback_estimate_agrees_with_the_real_tokenizer(budget): + """Both measurement paths must land on the same side of the soft budget.""" + if not _has_tiktoken(): + pytest.skip("tiktoken is not installed here, so there is no measurement to compare against; " + "install it with `python -m pip install tiktoken`") + text = SKILL_MD.read_text(encoding="utf-8") + + real, how_real = budget.token_estimate(text) + assert "tiktoken" in how_real, how_real + + # Force the fallback path by handing the module a tokenizer that cannot be imported. + saved = sys.modules.get("tiktoken") + sys.modules["tiktoken"] = None # `import tiktoken` raises here + try: + fallback, how_fallback = budget.token_estimate(text) + finally: + if saved is not None: + sys.modules["tiktoken"] = saved + else: + sys.modules.pop("tiktoken", None) + + assert "estimate" in how_fallback, how_fallback + drift = abs(real - fallback) / max(1, real) + assert drift < 0.05, ( + "the fallback is %.1f%% away from tiktoken (%d vs %d): recalibrate TOKENS_PER_CHAR " + "against a fresh measurement rather than a guess" % (100 * drift, fallback, real)) + assert (real <= budget.TOKEN_SOFT) == (fallback <= budget.TOKEN_SOFT), ( + "the two paths straddle the soft budget (%d real / %d fallback vs %d): the gate would " + "answer differently on CI than on a developer machine" + % (real, fallback, budget.TOKEN_SOFT)) + + +def test_the_fallback_constant_is_the_calibrated_one(budget): + """Pin the constant to a measurement: `chars / tokens` on SKILL.md, recomputed here.""" + text = SKILL_MD.read_text(encoding="utf-8") + real, how = budget.token_estimate(text) + if "tiktoken" not in how: + pytest.skip("no tiktoken available, so there is no measurement to calibrate against") + expected = len(text) / real + assert abs(budget.TOKENS_PER_CHAR - expected) < 0.25, ( + "TOKENS_PER_CHAR is %.3f but this SKILL.md measures %.3f characters per token -- the " + "document changed, so recalibrate" % (budget.TOKENS_PER_CHAR, expected)) + + +def test_an_index_row_past_the_hard_limit_fails_rather_than_warns(budget): + """The hard index-row limit must escalate, or the limit it names is decorative. + + It read `'warn' if hard else 'warn'` -- both branches identical -- so `INDEX_ROW_HARD` was + defined, computed, and unable to fail anything: an index row could grow without bound while + every gate stayed green. That is the same failure mode this file exists for, one level up: the + measurement happened and the verdict did not follow. It was found by injecting an over-long row + and requiring the gate to fail, which is the only way a dead limit shows itself -- a gate that + has only ever been seen passing is not evidence that it can see anything. + """ + def levels(desc_len): + lines = [ + "## Symptom index", + "| What you observe | Load first |", + "| a row | %s |" % ("y" * desc_len), + ] + return {level for level, _ in budget.check_index_rows(lines)} + + assert levels(budget.INDEX_ROW_SOFT + 10) == {"warn"} + assert levels(budget.INDEX_ROW_HARD + 10) == {"fail"}, ( + "a row past INDEX_ROW_HARD (%d chars) must fail the gate, not warn about it" + % budget.INDEX_ROW_HARD) + +def test_the_corpus_count_ignores_a_bytecode_cache(budget, tmp_path, monkeypatch): + """A measurement that changes when a reader runs the suite measures the reader. + + The corpus note once counted scripts/__pycache__/*.pyc, so the same commit reported 27 scripts + on a machine that had run pytest and 16 on a clean checkout. The routing gate said 16 the whole + time; the disagreement between two gates about one tree is what surfaced it. + """ + refs = tmp_path / "references" + refs.mkdir() + (refs / "one.md").write_text("# one\n", encoding="utf-8") + scripts = tmp_path / "scripts" + scripts.mkdir() + (scripts / "one.py").write_text("print(1)\n", encoding="utf-8") + cache = scripts / "__pycache__" + cache.mkdir() + (cache / "one.cpython-314.pyc").write_bytes(b"\x00" * 10) + + monkeypatch.setattr(budget, "REFS", str(refs)) + monkeypatch.setattr(budget, "SKILL_DIR", str(tmp_path)) + notes = budget.check_corpus() + assert notes[0].startswith("corpus: 1 references + 1 scripts = 2 files"), notes[0]