awesome-android/apk-reverse
3
0
Fork 0
mirror of https://github.com/newliver666/apk-reverse.git synced 2026-09-28 21:56:59 +02:00
Code Issues Packages Projects Releases Wiki Activity

Fix the command gate's blind spot, and make --json actually be JSON

Browse Source 
A reviewer reported seven drifting commands in the evidence record. Reproduced, the report split
three ways, and the middle category was the useful one: the gate had been scoring them clean.

Not reproducible, with the measurement:

  tests/run_tests.py -q   runs. The wrapper forwards its arguments to pytest
                          (`args = list(argv[1:]) or ["-q"]`), and documents that in its own usage.
  dcc.py                  already written as `cd tools/_work/bench/repos/dcc` followed by
                          `python dcc.py ...` -- an external checkout, not a shipped script.

Real, and fixed here: seven commands in two evidence records named a **workbench** script with no
path at all (`analyze_pair.py`, `make_skeleton.py`, `ttd_attach.py`, `b2_measure.py`). The files
exist under `tools/`, which .gitignore excludes, so a reader copying one from the repository root
gets "No such file" -- and two of those documents wrote the correct path elsewhere, so they
disagreed with themselves.

**The actual defect was in `check_commands.py`, not in the seven lines.** `resolve_script()` tried
the document's own directory first, so a bare `analyze_pair.py` inside `docs/tool-verification/`
resolved to `tools/_work/bench/unpack/b3/analyze_pair.py` -- a real file -- and passed. Resolution
now follows the order a reader would try (repository root, then the skill's scripts, then the
document's own directory, which is what `../x.py` in a nested README legitimately means); a bare
name that only resolves under `tools/` is `unqualified-workbench`; and that class counts as **drift,
not as a skip**, because it is a one-token fix that a reader cannot run. The first attempt classified
it as a low-confidence warning, which would have left all seven in place.

One exception is honoured and pinned: a code block opening with `cd tools/_work/bench/repos/dcc` is
telling the reader where to stand, so a bare `python dcc.py` after it is correct.
`documented_commands()` now reports whether a `cd` preceded the command in the same block, and that
case is skipped as `workbench-after-cd`.

Two defects of this pass's own, found while fixing it:

  --json emitted the RESULT= token after the JSON document, so `--json | jq` failed with "Extra
  data". The token is gone from that path; the status is a field in the document, and the exit code
  carries the same meaning.
  The fix's own substitution left four literal backslashes before `$` in the two files it edited --
  a character a reader would copy by mistake. Removed, and verified absent.

`tests/integration/test_check_commands_gate.py` pins the resolution order, the drift classification,
the `cd` exception, and a clean pass over the committed tree, so this cannot come back silently.

Gates: check_commands 153 checked / 0 drift, check_repo 0 problem, check_refs 0 dangling / 0 warning,
check_routing coherent, check_budget 0 failure / 0 warning, pytest 613 passed / 158 skipped / 25
xfailed.
This commit is contained in:
inliver233
2026-09-22 02:18:40 +08:00
parent fe5bdee087
commit 1dc1d9b707
5 changed files with 210 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
check_commands.py
+54 -15
View File
@@ -485,9 +485,13 @@ def documented_commands(text):
for idx, raw in enumerate(lines, 1):
if FENCE_RE.match(raw):
if in_fence:
# Whether an earlier line in the same block changed directory. A block that opens
# with `cd tools/_work/bench/repos/dcc` tells the reader where to stand, so a bare
# `python dcc.py` after it is correct -- and must not be reported as unqualified.
chdir = any(re.match(r'\s*(\$\s*)?cd\s+\S', ln) for ln in block)
for off, cmd in enumerate(join_continuations(block)):
for sub in split_shell(cmd):
yield (block_start + off, sub)
yield (block_start + off, sub, chdir)
block, in_fence = [], False
else:
in_fence, block, block_start = True, [], idx + 1
@@ -499,7 +503,7 @@ def documented_commands(text):
body = m.group(1).strip()
if body.startswith(('python ', 'python3 ', 'py ')):
for sub in split_shell(body):
yield (idx, sub)
yield (idx, sub, True) # an inline mention has no block context to honour
def find_script_token(tokens):
@@ -541,8 +545,20 @@ def _workbench_names():
return _WORKBENCH_INDEX['names']
def resolve_script(token, doc_path, include_workbench):
"""Where a documented script path points, or why it cannot be followed."""
def resolve_script(token, doc_path, include_workbench, chdir=False):
"""Where a documented script path points, or why it cannot be followed.
The order matters, and getting it wrong made this gate blind to a real class of drift: an
earlier revision resolved a relative path against the **document's own directory first**. A
bare `python analyze_pair.py` written inside `docs/tool-verification/` therefore resolved to
`tools/_work/bench/.../analyze_pair.py` -- a real file, hidden behind a gitignored directory --
and was scored `ok`. A reader who copies that command from the repository root gets
"No such file", which is the failure this gate exists to prevent.
So the roots are tried in the order a reader would: repository root, then the skill's script
directory, and only then the document's own directory (which is what `../x.py` in a nested
README legitimately means). A hit under `tools/` is a workbench artifact either way.
"""
norm = token.replace('\\', '/')
if '...' in norm:
# `python .../vmp_diff_harness.py compare ...`: the record elides the path on
@@ -552,27 +568,30 @@ def resolve_script(token, doc_path, include_workbench):
if os.path.isabs(token):
cands.append(token)
else:
# relative to the document that wrote it, which is how `../x.py` in a nested
# README reads; the repository-rooted and skill-rooted forms follow
cands.append(os.path.join(os.path.dirname(doc_path), norm))
cands.append(os.path.join(ROOT, norm))
if not norm.startswith('skills/'):
cands.append(os.path.join(SKILL_DIR, norm))
if not norm.startswith(('skills/', 'scripts/')):
cands.append(os.path.join(SCRIPT_DIR, norm))
cands.append(os.path.join(os.path.dirname(doc_path), norm))
for c in cands:
if os.path.isfile(c):
real = os.path.realpath(c)
tools_root = os.path.realpath(os.path.join(ROOT, 'tools')) + os.sep
if real.startswith(tools_root) and not include_workbench:
# reached by `../../tools/_work/...`: still a workbench artifact, and
# `.gitignore` excludes tools/, so a reader cannot open it either
# reached by `../../tools/_work/...`, or by a bare name that only exists there:
# still a workbench artifact, and `.gitignore` excludes tools/, so a reader who
# copies the command cannot open it
return None, 'workbench'
return real, 'ok'
if norm.startswith('tools/') or '/tools/' in norm:
return None, 'workbench'
if '/' not in norm and not include_workbench and norm in _workbench_names():
return None, 'workbench'
if chdir:
# The enclosing block already told the reader where to stand, so the bare name is
# correct. Reported as a skip rather than drift, and distinguishable in the summary.
return None, 'workbench-after-cd'
return None, 'unqualified-workbench'
if norm in EXTERNAL_SCRIPTS:
return None, 'external'
return None, 'missing-script'
@@ -607,7 +626,7 @@ def _allowed_positionals(spec):
def command_findings(doc_path, text, spec_cache, include_workbench):
findings = []
doc_rel = os.path.relpath(doc_path, ROOT)
for line_no, cmd in documented_commands(text):
for line_no, cmd, chdir in documented_commands(text):
cmd = cmd.strip()
if not cmd:
continue
@@ -634,9 +653,23 @@ def command_findings(doc_path, text, spec_cache, include_workbench):
continue
token = tokens[idx]
path, why = resolve_script(token, doc_path, include_workbench)
path, why = resolve_script(token, doc_path, include_workbench, chdir)
if path is None:
if why.startswith('workbench'):
if why == 'unqualified-workbench':
# A bare `python analyze_pair.py` where that name only exists under `tools/`.
# Reported as **drift, not a skip**: the reader who copies it from the repository
# root gets "No such file", and the fix is one path qualifier. This class was
# invisible until the resolver stopped preferring the document's own directory --
# the file existed *somewhere*, so the command scored `ok`.
kind = 'drift'
findings.append({'doc': doc_rel, 'line': line_no, 'command': cmd,
'kind': 'unqualified-workbench', 'script': token,
'suggest': ['a path that resolves from the repository root, e.g. '
'`tools/.../%s`' % token],
'detail': 'bare script name that only exists under tools/ '
'(gitignored): a reader cannot run it'})
continue
if why == 'workbench':
kind = 'workbench'
elif why == 'external':
kind = 'external'
@@ -867,7 +900,11 @@ def main(argv=None):
print('RESULT=%s' % TOKENS['internal'])
return EXIT_INTERNAL
drift = [f for f in all_findings if f['kind'] in ('unknown-flag', 'missing-script')]
# `unqualified-workbench` counts as drift: the command names a script that only exists under a
# gitignored directory and gives no path, so a reader who copies it fails. It is a one-token fix,
# which is exactly why it should be reported rather than skipped.
drift = [f for f in all_findings
if f['kind'] in ('unknown-flag', 'missing-script', 'unqualified-workbench')]
warnings = [f for f in all_findings if f['kind'] == 'warning']
unverifiable = [f for f in all_findings if f['kind'] == 'unverifiable']
workbench = [f for f in all_findings if f['kind'] == 'workbench']
@@ -914,7 +951,9 @@ def main(argv=None):
'findings': all_findings,
}
print(json.dumps(payload, indent=2, ensure_ascii=False))
print('RESULT=%s' % token)
# No `RESULT=` line here on purpose: appending one would make the stream invalid JSON for a
# caller doing `| jq .` or `json.loads(stdout)`, which is the entire point of `--json`. The
# status is a field in the document, and the exit code carries the same meaning.
return code
if args.fix_report:
docs/tool-verification/EXTENSION-benchmark-l1-l3.md
+3 -3
View File
@@ -427,7 +427,7 @@ for the OK button.
needs:
```
$ python ttd_attach.py --package <PKG> --activity sg.vantagepoint.uncrackable3.MainActivity \
$ python tools/_work/bench/l1l3/ttd_attach.py --package <PKG> --activity sg.vantagepoint.uncrackable3.MainActivity \
--js l3_read_crc.js --launch-delay 1.5 --seconds 20
[i] pre-attach main pid=7020 at 1.88s
[i] attached at +0.00s (probe loaded)
@@ -523,7 +523,7 @@ the expected original bytes before each write.
Patched condition:
```
$ python b2_measure.py --js l3_hook_patch_v2.js --tag PATCHED --probe-seconds 25
$ python tools/_work/bench/l1l3/b2_measure.py --js l3_hook_patch_v2.js --tag PATCHED --probe-seconds 25
[i] spawned pid=10000 (paused)
[i] resumed pid=10000
[0.04s][i] libfoo.so base=0x764b89d000 size=90112
@@ -545,7 +545,7 @@ I/Zygote( 1007): Process 10000 exited due to signal 6 (Aborted)
Control condition (identical reads and writes, original bytes written back):
```
$ python b2_measure.py --js l3_hook_control_v2.js --tag control_v2 --probe-seconds 22
$ python tools/_work/bench/l1l3/b2_measure.py --js l3_hook_control_v2.js --tag control_v2 --probe-seconds 22
[0.04s][CONTROL] 0x2ff0 unchanged (wrote back 40 1a 80 52 c0 03 5f d6)
[0.04s][CONTROL] 0x2ff8 unchanged (wrote back c0 0c 80 52 c0 03 5f d6)
[0.04s][CONTROL] 0x3080 unchanged (wrote back fd 7b bf a9 fd 03 00 91)
docs/tool-verification/EXTENSION-extraction-shell-bench.md
+2 -2
View File
@@ -44,7 +44,7 @@ A byte diff of the pair (`tools/_work/bench/unpack/b3/analyze_pair.py`) locates
difference:
```
$ python analyze_pair.py
$ python tools/_work/bench/unpack/b3/analyze_pair.py
len A=12536 len B=12536
first diff offset = 8 (0x8)
diff byte count = 6
@@ -83,7 +83,7 @@ the tail is nop-filled, so offsets, class table and string table are untouched a
header is re-fixed with `dexutil.fix_dex_header` (signature first, checksum last).
```
$ python make_skeleton.py src-r2pay.dex skel
$ python tools/_work/bench/unpack/b3/make_skeleton.py src-r2pay.dex skel
source: src-r2pay.dex
ctrl.dex fraction=0.00 mode=return emptied=0/5061
skel-25.dex fraction=0.25 mode=return emptied=1265/5061
docs/tool-verification/EXTENSION-reconstruction.md
+40 -1
View File
@@ -292,7 +292,46 @@ Defect 1 is the instructive one: a checker whose parser silently shortens a comm
for exactly the inputs it was written to reject, and nothing in its own output looks wrong. The
defence is not more careful reading, it is a fixture that is known to be bad.
### 2.6 What this check does not cover
### 2.6 A second blind spot, found by a reviewer reading this gate's output
A reviewer reported seven "drifting" commands. Reproducing them split the report three ways, and the
middle category is the one worth reading.
**Not reproducible.** `python tests/run_tests.py -q` runs: `run_tests.py` forwards what it is given
to pytest (`args = list(argv[1:]) or ["-q"]`), and its usage block documents exactly that. `dcc.py`
is written as `cd tools/_work/bench/repos/dcc` then `python dcc.py ...` — an external checkout, not a
script this repository ships.
**Real, and fixed.** Seven commands in two evidence records named a **workbench** script with no path
at all (`$ python analyze_pair.py`, `b2_measure.py`, `ttd_attach.py`, `make_skeleton.py`). Those files
exist under `tools/`, which `.gitignore` excludes, so a reader copying the command from the
repository root gets `No such file`. Two of those documents wrote the correct path elsewhere, so they
disagreed with themselves.
**The part that matters: this gate had been scoring those `ok`.** `resolve_script()` tried the
*document's own directory* first, so a bare `analyze_pair.py` inside `docs/tool-verification/`
resolved to `tools/_work/bench/unpack/b3/analyze_pair.py` — a real file — and passed. The fix is not
a stricter regex but a corrected resolution order plus a corrected classification:
1. Resolution now follows the order a **reader** would try: repository root, then the skill's script
directory, then the document's own directory (which is what `../x.py` in a nested README means).
2. A bare name that only resolves under `tools/` is `unqualified-workbench`, and it counts as
**drift, not a skip** — a one-token fix that a reader cannot run. A first attempt classified it as
a low-confidence warning, which would have left all seven in place.
3. The one honest exception is pinned too: a block that opens with `cd tools/_work/bench/repos/dcc`
tells the reader where to stand, so a bare `python dcc.py` after it is correct. That is skipped as
`workbench-after-cd`.
**Two defects of this pass's own, found while fixing it.** A `--json` run printed the `RESULT=` token
*after* the JSON document, so `--json | jq` failed with `Extra data`; the token is gone from that path
and the status is a field in the document instead. And the fix's own substitution left four literal
backslashes before `$` in the two files — a reader would have copied them.
`tests/integration/test_check_commands_gate.py` pins the resolution order, the drift classification,
the `cd` exception, and a clean pass over the committed tree, so this blind spot cannot return
silently.
### 2.7 What this check does not cover
- It validates **flags and (weakly) positional counts**, not semantics. A command with correct flags
and a wrong path still passes; so does a documented `--duration 14` where the useful value is
tests/integration/test_check_commands_gate.py
+111
View File
@@ -0,0 +1,111 @@
"""Regression tests for the documented-command gate itself.
`check_commands.py` is the gate that proves other documentation is runnable, so a blind spot in it
is worth more than a blind spot elsewhere: it turns "146 commands checked, 0 drift" into a claim the
repository cannot back. One such blind spot is pinned here, exactly as it was found.
**What was wrong.** `resolve_script()` tried the *document's own directory* first when resolving a
relative path. A bare `python analyze_pair.py` written inside `docs/tool-verification/` therefore
resolved to `tools/_work/bench/unpack/b3/analyze_pair.py` -- a real file, inside the gitignored
workbench -- and scored `ok`. Seven commands in two evidence files used that form while the same
files wrote the correct path elsewhere, and a reader copying one from the repository root gets
"No such file". The gate said the documentation was clean.
The tests below drive the resolver and the finding classifier directly rather than through the CLI,
so they stay fast and do not depend on the current state of the documentation.
"""
from __future__ import annotations
import sys
from pathlib import Path
import pytest
from conftest import REPO_ROOT
pytestmark = pytest.mark.integration
CHECK_COMMANDS = REPO_ROOT / "check_commands.py"
@pytest.fixture(scope="module")
def gate():
"""The gate as a module, so its internals can be driven without a subprocess."""
sys.path.insert(0, str(REPO_ROOT))
import importlib
return importlib.import_module("check_commands")
def _workbench_script_name(gate):
"""A `.py` basename that exists only under tools/, or skip if the workbench is absent."""
names = sorted(gate._workbench_names())
if not names:
pytest.skip("no workbench present (tools/ is gitignored and may not exist in a checkout)")
shipped = {p.name for p in (REPO_ROOT / "skills" / "apk-reverse" / "scripts").glob("*.py")}
root = {p.name for p in REPO_ROOT.glob("*.py")}
for name in names:
if name not in shipped and name not in root:
return name
pytest.skip("every workbench script name is also shipped; nothing to test with")
def test_a_bare_name_that_only_exists_in_the_workbench_is_drift(gate):
"""The exact defect: the name exists *somewhere*, which used to be enough to pass it."""
name = _workbench_script_name(gate)
doc = str(REPO_ROOT / "docs" / "tool-verification" / "EXTENSION-extraction-shell-bench.md")
path, why = gate.resolve_script(name, doc, include_workbench=False)
assert path is None
assert why == "unqualified-workbench", why
text = "```\n$ python %s --flag\n```\n" % name
findings = gate.command_findings(doc, text, {}, False)
kinds = [f["kind"] for f in findings]
assert "unqualified-workbench" in kinds, findings
def test_a_path_qualified_workbench_reference_is_a_skip_not_drift(gate):
"""`tools/_work/...` is deliberate in an evidence record: a skip, reported with its reason."""
name = _workbench_script_name(gate)
doc = str(REPO_ROOT / "docs" / "tool-verification" / "EXTENSION-extraction-shell-bench.md")
_path, why = gate.resolve_script("tools/_work/somewhere/%s" % name, doc, include_workbench=False)
assert why == "workbench"
def test_a_bare_name_after_a_cd_in_the_same_block_is_not_drift(gate):
"""A block that says `cd tools/_work/bench/repos/dcc` tells the reader where to stand."""
name = _workbench_script_name(gate)
doc = str(REPO_ROOT / "docs" / "tool-verification" / "EXTENSION-java2c.md")
text = "```\n$ cd tools/_work/bench/repos/dcc\n$ python %s --no-build\n```\n" % name
findings = gate.command_findings(doc, text, {}, False)
kinds = [f["kind"] for f in findings]
assert "unqualified-workbench" not in kinds, findings
assert "workbench-after-cd" in kinds, findings
def test_a_shipped_script_resolves_from_the_repository_root(gate):
"""The order that matters: root, then the skill's scripts, then the document's own directory."""
doc = str(REPO_ROOT / "docs" / "tool-verification" / "README.md")
path, why = gate.resolve_script("check_repo.py", doc, include_workbench=False)
assert why == "ok" and path and path.endswith("check_repo.py")
path, why = gate.resolve_script("skills/apk-reverse/scripts/scan_leaks.py", doc,
include_workbench=False)
assert why == "ok" and path and path.endswith("scan_leaks.py")
def test_the_committed_tree_reports_no_drift(gate, tmp_path):
"""The gate's own verdict on this repository, run in-process so it cannot drift silently."""
docs = gate.collect_docs(None)
findings = []
cache = {}
for doc in docs:
try:
text = Path(doc).read_text(encoding="utf-8")
except OSError:
continue
findings.extend(gate.command_findings(doc, text, cache, False))
drift = [f for f in findings
if f["kind"] in ("unknown-flag", "missing-script", "unqualified-workbench")]
detail = "\n".join(" %s:%s %s (%s)" % (f["doc"], f["line"], f["command"], f["kind"])
for f in drift[:20])
assert not drift, "documented commands that cannot run:\n%s" % detail