mirror of
https://codeberg.org/reverse-skill-lab/apk-reverse
synced 2026-11-15 20:44:22 +08:00
Reconstruction pass: make the gates tell the truth, make the package self-contained, keep the one-hop symptom index
An external audit of this repository landed on four structural findings, and this pass is the
response to them. Everything below was re-measured on this tree before being acted on; two of the
audit's claims needed a correction of my own, and both corrections are noted in place.
**The gates were optimistic, and one of them did not read content at all.**
doctor.py the capability table was hard-coded (`repack + sign = bool(java) and python`), so a
machine with a JDK and no zipalign/apksigner was told it could re-sign; the script
inventory was a hand-written list of 28 against 51 files on disk, omitting the nine
most important scripts. It now scans the directory (`registered/checked = 52 / 52`,
unjudgeable entries counted as `unknown`, never skipped) and resolves capability as a
real requires-closure, reporting BLOCKED with the missing piece and an installable
next step. Measured: the old doctor reports `[OK] repack + sign` on a JDK-only PATH
where keytool, jarsigner, zipalign, apksigner and all eight smali jars are absent.
The report is allowed to look bad; that is the point.
check_commands.py (new)
anchor checks prove a section exists, never that a command runs. This parses each
script's argparse table with `ast` and checks every documented invocation against it.
146 commands / 41 interfaces, 0 drift. It caught `sig_probe.py --live --pkg`, which
does not exist -- the reference had been wrong since it was written.
check_routing.py (new)
the symptom index now has one home (`SKILL.md`, because symptom -> file must stay one
hop) and the inventory has another (`references/routing.md`). This gate proves the
mirror agrees, that every reference file is named, and that every script is named.
**so_constpatch.py was corrupting APKs it rebuilt.** The writer copied filename and timestamp onto a
fresh ZipInfo and recompressed the whole archive: 435-entry real package, `extra` lost on 205 entries,
`external_attr` rewritten on all 435, 132 untouched entries recompressed, and `resources.arsc` pushed
to an unaligned offset. Android's own `zipalign -c -v 4` fails the old output and passes the new one;
a 26-case fixture suite pins it, and `extractNativeLibs="false"` is now refused by default with a
pointer at `repack.py`. Fixing it exposed the same defect in `repack.py`, where `^lib/[^/]+\.so$`
cannot match `lib/arm64-v8a/*.so` -- so its alignment handling had never once fired on a real APK.
**The package was not self-contained.** 59 references pointed at the repository root, which `npx
skills add` does not install. They now converge on `references/evidence-summary.md` (capability,
conclusion, strength, evidence that ships) plus `evidence/` with the machine-readable companions: 8
references remain and each is explicitly labelled "repository root, not shipped". A drill in which
the new files were copied to an isolated directory and asked three questions answered all three.
**SKILL.md: 572 lines / 15,733 tokens -> 398 lines / ~10,900 tokens**, without losing the two things
that had to stay. The symptom index remains inline; the reference and script indexes moved to the
on-demand routing file. The freed budget went into making the rules *stronger*, not shorter: the
constraints section is now a table of thirteen non-negotiable constraints each paired with the check
that catches a violation, and the tooling obligations are instructions whose violation is a defect
(hedging language in SKILL.md: 1 line, down from a paragraph of hedged advice). Long references gained
a navigation head so a reader can decide from the top whether to load one.
**New: an executable test layer.** `tests/` (unit, CLI contract, no-device integration) plus
`pytest.ini`, `tests/run_tests.py`, a minimal CI job, and `evals/evals.json` written but never run --
stated as such in the file. 608 passed / 158 skipped / 25 xfailed. The 25 xfails are a work order, not
a pass: thirteen scripts report a missing input by exiting 1 instead of 2, seven let a traceback
escape, four treat `--help` as an error, and `preflight --json` lacks a `status` field. A regression
row for `dex_patch_bytes.py` reproduces a previous artifact byte-for-byte
(`67ae1e58e7a44777e5ac8f9f823ab21e…`) and additionally pins the four bytes that may move.
**Two real defects the new tests found in the tooling itself.** `check_repo.py` decoded subprocess
output with the locale encoding, so on a GBK console a script printing an em dash in its usage line
was reported as crashed; it now decodes UTF-8 explicitly. And a CLI contract test drove a *real
attached device* through `coldstart.py` -- logcat, screenshots, five orphan `adb logcat` processes --
which is fixed twice over: child processes run in a private temporary cwd, and the fifteen
device-bound scripts are never executed beyond `--help`/argument parsing.
**Quoting on the device side is now one implementation.** `devsh.su` escaped only double quotes, so
`;`, `$( )` and backticks in a caller-supplied value still reached the device shell while
`coldstart.Device.shell` quoted correctly. Both use `scripts/device_shell.py` now: POSIX single-quote
escaping, argv arrays by default, and identifier validation that refuses a malformed package, path or
component with exit 2 instead of a traceback. The integration test that used to pin the *defect* (it
asserted `su -c "…"` with `\"` escapes) now asserts the property, across five metacharacter payloads.
Gates: check_repo 0 problems (including the content-reading identity scan over git ls-files),
check_refs 0 dangling / 0 warning, check_routing coherent (53 symptom / 45 reference / 54 script rows),
check_budget 0 failure / 0 warning with the always-loaded body now measured in tokens rather than in
"narrative lines with the index excluded", check_commands 146 checked / 0 drift, pytest 608 passed.
Not done, and stated rather than implied: CI has never run on a runner; the evals have never been
executed; the 25 xfails are open; no device-side install/launch was performed in this pass.
This commit is contained in:
inliver233
@@ -0,0 +1,521 @@
|
||||
# Extension verification — truthful environment gates, documented-command validation, and APK rebuild safety
|
||||
|
||||
Covers the P0 pass that replaced the optimistic half of `skills/apk-reverse/scripts/doctor.py`
|
||||
with a probed capability registry, added `check_commands.py` at the repository root, and — in the
|
||||
same round, by teammate `zip-safety` — replaced the APK-rebuild writer inside
|
||||
`skills/apk-reverse/scripts/so_constpatch.py`. Strength labels are the ones this directory's
|
||||
README defines: **observed** (reproduced here, exact command and output recorded), **inferred**
|
||||
(follows from observed facts, the step itself was not executed), **unverified** (assumed or
|
||||
reported elsewhere, not independently reproduced).
|
||||
|
||||
Two gates depend on the material below, which is why the report was allowed to become worse
|
||||
looking: G2 (environment gating) reads the capability verdicts, and the documented-command check
|
||||
is what keeps a reader from following a page instead of the tool. "Documented command" below means
|
||||
a script invocation written in a code block or inline backtick in a markdown file — the shape
|
||||
`check_commands.py` extracts and validates.
|
||||
|
||||
Reference environment for this pass: Windows 11 / AMD64, Python **3.14.0**, JDK **17.0.4.1**
|
||||
(`java`/`javac` only, on the Oracle launcher shim path), `adb` 1.0.41 with one attached
|
||||
`arm64-v8a` device at API 30, Frida **16.7.19** host package, capstone present, Android
|
||||
build-tools unpacked at a local directory but **not** on `PATH`. Workbench artifacts live in
|
||||
`tools/_work/truth-gates/` and `tools/_work/zip-safety-out/` (not part of the skill; `tools/` is
|
||||
gitignored, so those paths are an operation record rather than something a reader can open).
|
||||
|
||||
---
|
||||
|
||||
## 0. Summary of what this pass established
|
||||
|
||||
| Claim | Label |
|
||||
|---|---|
|
||||
| The committed `doctor.py` registered **28** scripts against **51** files actually in `scripts/` (46 `.py` + 5 `.js`) | observed |
|
||||
| Its capability table derived `repack + sign` and `smali round-trip` from `java` alone; on a JDK-only `PATH` it reported **both as OK** while `keytool`, `jarsigner`, `zipalign`, `apksigner` and all eight smali jars were absent | observed |
|
||||
| The rebuilt `doctor.py` scans the directory, so `registered/checked = N / N` equals the file count; on this host `52 / 52` (47 `.py` + 5 `.js`, the new `capabilities.py` included) | observed |
|
||||
| Capability verdicts now come from a real `requires` closure with an executable next action and a cost estimate; 16 capabilities are resolved, and the same JDK-only host reports `repack_signed` and `smali_roundtrip` **BLOCKED** | observed |
|
||||
| `check_commands.py` reads each script's `argparse` interface with `ast` (no script executed to learn its interface) and validates **139** documented commands across 76 markdown files against 39 interfaces, with 0 drift after the one known drift was fixed | observed |
|
||||
| The drift the checker was written for — `sig_probe.py --live --pkg <pkg>` — is reproduced **from `git show HEAD:`** of the pre-fix page and is reported at the right line | observed |
|
||||
| The pre-existing `so_constpatch.py` APK writer dropped per-entry zip metadata and lost `resources.arsc` alignment; the replacement keeps metadata byte-for-byte and the alignment gate passes | observed |
|
||||
| `zipalign -c -v 4` from the Android build-tools agrees with that verdict independently (old writer `rc=1 FAILED`, new writer `rc=0`), re-checked in this pass on artifacts produced by the other teammate | observed |
|
||||
| `repack.py:_needs_alignment` uses `^lib/[^/]+\.so$`, which never matches the real `lib/<abi>/x.so` layout, so its alignment filler is dead code on real APKs | observed (reported, **not** fixed — outside every current write scope) |
|
||||
| A real `pm install` of any rebuilt artifact | not done — no install was attempted in this pass |
|
||||
|
||||
---
|
||||
|
||||
## 1. The doctor was optimistic, and a gate trusted it
|
||||
|
||||
### 1.1 A hand-written inventory that could only go stale
|
||||
|
||||
The committed version carried `script_dependencies()` returning a literal dict of 28 entries. The
|
||||
directory held 51 files. The 23 absent from the table were not peripheral: they included
|
||||
`dex_patch_bytes.py`, `dex_find_insn.py`, `dex_dump_validate.py`, `dex_mem_scan.py`, `dexutil.py`,
|
||||
`protobuf_decode_raw.py`, `java2c_probe.py`, `vmp_diff_harness.py` and `scan_leaks.py` — nine of the
|
||||
scripts this skill exists for. The summary line read `scripts runnable here: 28 / 28`, so the report
|
||||
looked like complete coverage of a set that was less than half the set.
|
||||
|
||||
The replacement cannot drift in that direction, because the list *is* the directory:
|
||||
|
||||
```
|
||||
$ python skills/apk-reverse/scripts/doctor.py --scripts
|
||||
--- scripts ---
|
||||
directory: E:\apk-reverse\skills\apk-reverse\scripts
|
||||
registered/checked = 52 / 52 (.py=47, .js=5)
|
||||
ok=33 partial=13 blocked=6 unknown=0
|
||||
```
|
||||
|
||||
A file whose dependency set cannot be decided is printed as `UNKNOWN` and still counted in the
|
||||
denominator; `unknown=0` here is a measurement, not an assumption. Dependencies come from an `ast`
|
||||
pass over imports checked against `sys.stdlib_module_names`, plus a narrow scan for host tools
|
||||
(`resolve_tool('java')`, the program of a `subprocess` list) against a known-tool set. Modules are
|
||||
never imported to probe them, because importing the probe target is itself a side effect — `frida`
|
||||
starts threads and enumerates devices at import time.
|
||||
|
||||
### 1.2 The formula that ignored the toolchain — decisive reproduction
|
||||
|
||||
`doctor.py:319-320` (committed):
|
||||
|
||||
```python
|
||||
caps['repack + sign'] = bool(tools['java']['path']) and py['ok']
|
||||
caps['smali round-trip'] = bool(tools['java']['path']) and py['ok']
|
||||
```
|
||||
|
||||
Neither ability follows from `java`. To show that the mechanism was wrong rather than merely lucky,
|
||||
the committed script was extracted from git and run with a `PATH` holding a JDK and nothing else:
|
||||
|
||||
```
|
||||
$ git show HEAD:skills/apk-reverse/scripts/doctor.py > tools/_work/truth-gates/doctor-HEAD.py
|
||||
$ set PATH=C:\Program Files\Common Files\Oracle\Java\javapath;C:\Windows\System32;C:\Windows;C:\Python314
|
||||
$ python tools/_work/truth-gates/doctor-HEAD.py
|
||||
|
||||
--- capabilities ---
|
||||
[OK ] static (dex/zip/strings)
|
||||
[OK ] repack + sign <-- no keytool, no zipalign, no signer on this PATH
|
||||
[OK ] smali round-trip <-- none of the eight jars exist on this host
|
||||
[OK ] native ELF patching
|
||||
[-- ] jadx decompile
|
||||
...
|
||||
```
|
||||
|
||||
The rebuilt script, on the **same** restricted `PATH`:
|
||||
|
||||
```
|
||||
[BLOCKED] repack_signed Repack, align and sign an APK
|
||||
missing : tool:keytool -- not on PATH or APKREV_TOOLS
|
||||
next: the directory on PATH (C:\Program Files\Common Files\Oracle\Java\javapath)
|
||||
is a launcher shim, not a full JDK bin/: put %JAVA_HOME%\bin first, or
|
||||
install a JDK - a JRE ships no keytool [~2 min, unverified]
|
||||
missing : tool:zipalign -- not on PATH or APKREV_TOOLS
|
||||
next: build-tools are already unpacked here but are not on the search path:
|
||||
set APKREV_TOOLS=E:\tools\android-14;E:\tools\bin (found: ...) [~0 min, observed]
|
||||
missing : artifact:signer -- tool:apksigner (not on PATH or APKREV_TOOLS);
|
||||
jar:uber-apk-signer (uber-apk-signer.jar not found)
|
||||
reduced : tool:jarsigner -- not on PATH or APKREV_TOOLS
|
||||
[BLOCKED] smali_roundtrip Smali disassemble / assemble round-trip
|
||||
[OK ] repack_unsigned Repack an APK without signing (--no-sign path)
|
||||
|
||||
ok=5 partial=1 blocked=10 (of 16 capabilities)
|
||||
```
|
||||
|
||||
`repack_unsigned` staying OK is deliberate and true: `repack.py --no-sign` is `zipfile` plus the
|
||||
STORED-entry rules and needs no Java tooling at all. A gate that reported the whole file unusable
|
||||
would have been a pessimism of its own.
|
||||
|
||||
### 1.3 The closure is the point, not the atom list
|
||||
|
||||
`capabilities.py` is the single registry: 16 capabilities, each a `name`, a `note`, the scripts it
|
||||
serves, `required` atoms, `optional` atoms, and `depends_on` capabilities that are expanded
|
||||
recursively. `repack_signed` depends on `repack_unsigned`, so it cannot be OK while the repacking it
|
||||
is built on is not. Atoms cover the four kinds that actually decide something here: an importable
|
||||
module, a tool resolvable on `PATH` or `APKREV_TOOLS`, a jar set, a device property (present,
|
||||
rooted, frida-server running), a repository file, and one **input artifact** that no host can
|
||||
install — the Dart AOT snapshot dump, which is why `dart_aot_full` reports BLOCKED with
|
||||
`build blutter … or run the pinned aotopsy front end` rather than a download link.
|
||||
|
||||
`status ∈ ok | partial | blocked`. A missing *required* atom blocks. A missing *optional* atom
|
||||
degrades: `static_native` stays usable without `capstone` and names the part that is not
|
||||
(`svc_scan.py` refuses to run without it; the caller index in `dart_disasm.py` works).
|
||||
|
||||
Two probes were worth getting right rather than guessing:
|
||||
|
||||
- A JDK whose launcher shim is on `PATH` (`...\Oracle\Java\javapath`) exposes `java` and `javac`
|
||||
but not `keytool` or `jarsigner`. The registry probes those separately and, on failure, says the
|
||||
shim directory is a shim instead of reporting a missing JDK.
|
||||
- Build-tools present but unexposed are found by searching the usual SDK locations plus a
|
||||
conventional `tools/` directory, and the next action becomes the exact assignment
|
||||
(`set APKREV_TOOLS=E:\tools\android-14;E:\tools\bin`) with a measured cost of `~0 min`, rather
|
||||
than "install build-tools".
|
||||
|
||||
That last hint was verified end to end: with `APKREV_TOOLS` set, `repack_signed` loses the
|
||||
`zipalign` and `signer` blockers and keeps only `keytool` — which is the honest residual on this
|
||||
host, because the JDK really is a shim install.
|
||||
|
||||
### 1.4 Cost estimates carry their own strength
|
||||
|
||||
Every install hint carries `est_minutes` with a `basis`. `observed` means measured in this
|
||||
repository: the `--unsafe-rebuild`-independent blutter build is **≈78 s** end to end
|
||||
(`TOOL-VERDICTS.md`), and exposing an already-unpacked build-tools directory is 0 minutes because
|
||||
the search found it. Everything else is labelled `unverified` and is a planning aid only: a guessed
|
||||
ten minutes printed as a measurement is the same class of error as a capability verdict derived
|
||||
from the wrong tool.
|
||||
|
||||
Per-capability `est_basis` is `mixed` when the missing atoms disagree, which is what makes
|
||||
`repack_signed` read `~12 min, mixed` rather than an authoritative-looking single number.
|
||||
|
||||
### 1.5 Exit codes and the token
|
||||
|
||||
`doctor.py` and `capabilities.py` both follow this round's convention: `0` = nothing blocked
|
||||
(`partial` does not fail a gate, and the counts are printed so it cannot hide), `3` = at least one
|
||||
capability blocked, `2` = usage, `4` = internal. The last line is `RESULT=env_ok` /
|
||||
`env_partial` / `env_blocked` / `usage_error` / `internal_error`, and the usage-error path keeps the
|
||||
token too — `argparse` would otherwise exit 2 with nothing a caller can branch on.
|
||||
|
||||
`--json` carries `{status, exit_code, capability?, evidence[], warnings[], next_action?}` plus, for
|
||||
compatibility, the keys the old report used (`python`, `tools`, `jars`, `device`, `capabilities`,
|
||||
`scripts`). `capabilities` changed shape on purpose — the old flat `{name: bool}` map *was* the
|
||||
optimism — and a `capabilities_legacy` map gives the same information as `{id: "ok"|"partial"|"blocked"}`
|
||||
for any consumer that only wants the verdict.
|
||||
|
||||
---
|
||||
|
||||
## 2. `check_commands.py` — the commands a reader is told to run
|
||||
|
||||
### 2.1 What it reads, and from where
|
||||
|
||||
`check_repo.py` proves that a referenced path resolves and every script answers `--help`. Neither
|
||||
can see a documented command whose flags no longer exist, and the repository shipped exactly one for
|
||||
a long time. `check_commands.py` closes that gap by reading each script's real interface with `ast`:
|
||||
|
||||
- `ArgumentParser()` / `add_argument()` calls are parsed statically, so **no script is executed to
|
||||
learn its interface**;
|
||||
- `add_subparsers()` / `add_parser('name')` are followed, and a sub-parser's flags are attributed
|
||||
to that sub-command rather than merged into the parent (one variable reused across several
|
||||
sub-parsers is handled, and the parameters are then attributed to the group as a whole);
|
||||
- `action=store_true|store_false|count|help|version` and `nargs=0` mean the flag consumes no value;
|
||||
- a script with no `argparse` at all (a hand-rolled `sys.argv` parser: `smtool.py`,
|
||||
`dex_strpatch.py`) falls back to parsing its own `--help` output, and `smtool.py`'s `--cp` is
|
||||
recovered that way. The probe runs in a **throwaway directory**, because a probe must not leave
|
||||
anything behind — a script that reads its first argument as an output directory would otherwise
|
||||
create one named after the flag.
|
||||
|
||||
Documented commands are extracted from fenced code blocks and inline backticks in `README.md`,
|
||||
`skills/**/*.md` and `docs/**/*.md`. A documented line is split on `&&`, `||`, `;` and `|`, and a
|
||||
redirection truncates the segment: `python check_repo.py && python check_refs.py` is two commands,
|
||||
not one with two extra positionals. `<PKG>`-style placeholders are kept as single tokens, because
|
||||
reading `<pkg>` as a shell input redirect truncates the command at `--pkg` — and a truncated
|
||||
command checks clean, which is how the checker would have missed the one drift it was built for.
|
||||
|
||||
### 2.2 Allowance rules that keep it honest rather than noisy
|
||||
|
||||
- argparse accepts a **unique prefix** of a long option, so `--max` passes for `--max-depth`; an
|
||||
*ambiguous* prefix is reported.
|
||||
- `-h` / `--help` are always accepted.
|
||||
- A flag that needs a value does **not** consume an option-looking token; argparse reports
|
||||
"expected one argument" instead. This is the rule that makes `--live --pkg <pkg>` report `--pkg`
|
||||
rather than swallow it as the value of `--live`.
|
||||
- Positional counts are computed from the declared positionals (with `nargs=None|int|'?'|'*'|'+'`)
|
||||
and are reported as **warnings**, never as drift: a static count can be thrown off by a
|
||||
placeholder or an elision, so it does not get to fail a gate. It is skipped entirely when any
|
||||
option has a variable `nargs` or the interface came from `--help`.
|
||||
|
||||
### 2.3 The drift it exists for, reproduced from git
|
||||
|
||||
The page was already fixed by the time this checker ran, so the pre-fix revision is taken from git
|
||||
and checked as a file:
|
||||
|
||||
```
|
||||
$ git show HEAD:skills/apk-reverse/references/signature-derived-keys.md > tools/_work/truth-gates/sig-derived-HEAD.md
|
||||
$ python check_commands.py --fix-report --path tools/_work/truth-gates/sig-derived-HEAD.md
|
||||
tools\_work\truth-gates\sig-derived-HEAD.md:61 sig_probe.py unknown flag --pkg closest known: --apk cmd: python scripts/sig_probe.py --live --pkg <pkg>
|
||||
RESULT=commands_drift
|
||||
```
|
||||
|
||||
Line 61, the right script, and the correct suggestion (`sig_probe.py` takes `--live PKG` and has
|
||||
never had `--pkg`). A second, synthetic fixture proves the check is not a single-rule special case:
|
||||
|
||||
```
|
||||
$ python check_commands.py --fix-report --path tools\_work\truth-gates\NEGATIVE-unknown-flag.md
|
||||
...:4 sig_probe.py unknown flag --pkg closest known: --apk cmd: python skills/apk-reverse/scripts/sig_probe.py --live --pkg <pkg>
|
||||
...:5 dex_classdiff.py unknown flag --original closest known: --max-list cmd: ... --original a.dex --patched b.dex
|
||||
...:5 dex_classdiff.py unknown flag --patched closest known: --max-list cmd: ... --original a.dex --patched b.dex
|
||||
...:7 skills/apk-reverse/scripts/does_not_exist.py unknown flag missing-script closest known: - cmd: python .../does_not_exist.py --x 1
|
||||
RESULT=commands_drift
|
||||
```
|
||||
|
||||
Exit code 1, one machine-readable line per finding, with the document line, the command, the
|
||||
unknown flag and the closest known flags — which is what `--fix-report` is for.
|
||||
|
||||
### 2.4 Coverage on the real repository
|
||||
|
||||
```
|
||||
$ python check_commands.py
|
||||
docs : 76 markdown file(s)
|
||||
scripts : 39 argument table(s) read, 2 without a readable table
|
||||
commands : 139 checked (139 clean, 0 drift, 0 warning)
|
||||
skipped : 24 workbench, 1 external, 1 quoted, 0 unverifiable, 30 not a script invocation
|
||||
RESULT=commands_ok
|
||||
```
|
||||
|
||||
`139 checked` counts every invocation resolved to a readable interface, not merely the ones with
|
||||
findings; a pass that reports only its findings cannot tell "clean" from "nothing looked at".
|
||||
Everything skipped is printed with its reason, and the reasons are of four kinds:
|
||||
|
||||
- **workbench** (24): the script lives under `tools/`, which `.gitignore` excludes, so a reader
|
||||
cannot open it and a missing file there is not drift. A bare filename that only exists under
|
||||
`tools/` is caught too, because those evidence records refer to workbench scripts by name.
|
||||
- **not a script invocation** (30): `python -m …`, `python -c "…"`, an elided `python ... --flag`,
|
||||
or an output line that merely starts with `python`.
|
||||
- **external** (1): `python scripts\init_env_win.py` in the blutter build record is a path inside
|
||||
*another* repository's checkout — the record's preceding line is `git clone --depth 1
|
||||
worawit/blutter`. Each such entry carries its reason in an auditable table that the report
|
||||
prints, because a silent allow-list is indistinguishable from a missed drift.
|
||||
- **quoted** (1): `FINDINGS.md` quotes `python scripts/sig_probe.py --live --pkg <pkg>` *as the
|
||||
defect it records*. It is evidence, not an instruction; the page's own sentence says the script
|
||||
has no `--pkg`. Same auditable-table treatment.
|
||||
|
||||
The two scripts with no readable table are listed by name. A script with no flags has nothing that
|
||||
can drift, so its invocations pass without being checkable — stated so the edge of the check is
|
||||
visible instead of implied.
|
||||
|
||||
### 2.5 Defects the negative tests found in the checker itself
|
||||
|
||||
Each of these produced a *false clean* on the pre-fix page, and each was found by testing the
|
||||
checker against a known-bad document rather than by reading its code.
|
||||
|
||||
| # | Defect | How it surfaced | Fix |
|
||||
|---|---|---|---|
|
||||
| 1 | `<pkg>` was read as a shell input redirection, truncating the command at `--pkg`; the truncated command then checked clean | the git-HEAD reproduction reported no drift at all | `<...>` with no interior space is treated as a placeholder token; `>`/`>>` still truncate |
|
||||
| 2 | A flag that takes a value consumed the *next* token unconditionally, so `--live --pkg` made `--pkg` the value of `--live` | the same reproduction, after fix 1 | an option-looking next token is not consumed, matching argparse's "expected one argument" |
|
||||
| 3 | The whole documented line was read as one argv, so `&&`, the second `python`, and a redirect target entered the positional count | 3 warnings about `check_repo.py && python check_refs.py` and `… --json > out.json` | lines are split on `&&`/`\|\|`/`;`/`\|` and truncated at a redirection |
|
||||
| 4 | `docs/**` records reach workbench scripts as `../../tools/_work/…`, which resolves to a real file, so they were reported as unverifiable repository scripts | 17 spurious entries | a resolved path under `tools/` is classified workbench, and paths are `realpath`-normalised so `dexpatch/../smtool.py` is one entry, not two |
|
||||
| 5 | `commands_checked` counted only commands *with findings*, so a clean run reported `3 checked` instead of 139 | comparing the summary against the extraction dump | every resolved invocation is emitted as a `clean` record and counted |
|
||||
| 6 | `--help` fallback probes ran with `cwd` at the repository root | reasoning about probe side effects, then confirmed | the probe runs in a temporary directory |
|
||||
|
||||
Defect 1 is the instructive one: a checker whose parser silently shortens a command reports success
|
||||
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
|
||||
|
||||
- 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
|
||||
different.
|
||||
- A script whose interface is neither declarative nor printable is outside the check; the two
|
||||
current cases are listed by name rather than folded into the pass count.
|
||||
- It does not verify that a command *works* — only that the script would not reject it as unknown
|
||||
arguments. Running the 139 commands remains a benchmark-pass activity.
|
||||
|
||||
---
|
||||
|
||||
## 3. APK rebuild safety in `so_constpatch.py` (teammate `zip-safety`; re-checked here)
|
||||
|
||||
### 3.1 What the committed writer did
|
||||
|
||||
```python
|
||||
# so_constpatch.py:204-218, before this round
|
||||
with zipfile.ZipFile(out_path, 'w', zipfile.ZIP_DEFLATED) as dst:
|
||||
for item in src.infolist():
|
||||
blob = new_data if item.filename == entry else src.read(item.filename)
|
||||
dst.writestr(zipfile.ZipInfo(item.filename, date_time=item.date_time),
|
||||
blob, item.compress_type)
|
||||
```
|
||||
|
||||
A new `ZipInfo` built from a filename and a timestamp carries nothing else, so `extra` and
|
||||
`external_attr` are lost; `create_system`, `internal_attr` and the version fields are reset; and
|
||||
every entry is re-compressed as DEFLATED, so a STORED `resources.arsc` may stop being STORED and
|
||||
lose its 4-byte data alignment. There was no `extractNativeLibs` check, no alignment check, and no
|
||||
pointer at `scripts/repack.py`.
|
||||
|
||||
### 3.2 What replaced it
|
||||
|
||||
The writer now emits the local headers and the central directory itself, because alignment is a
|
||||
property of the local header length that `zipfile` cannot express. Per entry it reproduces the raw
|
||||
local name bytes, both `extra` fields, `external_attr`, `internal_attr`, `create_system`,
|
||||
`create_version`, `extract_version`, the comment, the DOS time/date words and the general-purpose
|
||||
flags (the data-descriptor bit is cleared, since the size is known up front). An untouched STORED
|
||||
entry's compressed stream is copied verbatim in 1 MiB chunks, so its CRC and `compress_size` are
|
||||
identical by construction rather than by luck. Only the named entry is ever re-compressed, and only
|
||||
if it was DEFLATED. Alignment padding goes into a private local extra record (`0xCA`) so the
|
||||
original extra field stays a byte-for-byte prefix.
|
||||
|
||||
### 3.3 The offset that matters — a correction to state plainly
|
||||
|
||||
The alignment contract is the **data offset** of the entry, not its local-header offset. This
|
||||
matters because the two differ by the header length plus the name, and real archives routinely have
|
||||
an unaligned header offset:
|
||||
|
||||
```
|
||||
r2pay (input, 435 entries): resources.arsc STORED data offset 0x3cec08 header_offset 0x3cebce
|
||||
data offset % 4 == 0 OK header_offset % 4 == 2
|
||||
```
|
||||
|
||||
`zipalign`, `apksigner` and `repack.py`'s own `check_alignment` all test the data offset. So the
|
||||
rule this tool enforces is: `resources.arsc` must be STORED and its **data offset must be a
|
||||
multiple of 4**; any other STORED entry that arrived aligned must still be aligned after the
|
||||
rebuild. The alignment predicate differs from `repack.py`'s in one substantive way —
|
||||
|
||||
```
|
||||
always : resources.arsc (Android R+ install gate)
|
||||
always : STORED lib/**/*.so (extractNativeLibs=false maps them in place)
|
||||
preserved: any other STORED entry already at a 4-byte data offset
|
||||
never : DEFLATED entries, and STORED entries that arrived unaligned
|
||||
```
|
||||
|
||||
— and it uses `^lib/.+\.so$` where `repack.py` uses `^lib/[^/]+\.so$`. The latter matches
|
||||
`lib/x86/libfoo.so` but **not** `lib/arm64-v8a/libfoo.so`, because the ABI directory is a second
|
||||
path segment: on a real APK that pattern matches nothing, so the alignment filler it guards would
|
||||
never fire. That is a live defect in `repack.py` (`_needs_alignment`, and the
|
||||
`check_alignment`/`zip_report` paths that inherit the predicate), reported rather than fixed
|
||||
because `repack.py` is in no one's write scope this round. It is worth fixing before the refusal
|
||||
message tells users to switch to that tool.
|
||||
|
||||
### 3.4 Evidence, fixture level
|
||||
|
||||
```
|
||||
arsc_aligned.apk (7 entries)
|
||||
OLD writer vs INPUT NEW writer vs INPUT
|
||||
extra differ 7/7 (nothing but offset)
|
||||
eattr differ 7/7
|
||||
csys differ 7/7
|
||||
ver_made differ 7/7
|
||||
untouched entries with changed CRC/size : 2 untouched entries with changed CRC/size : 0
|
||||
resources.arsc: STORED @0x3a3 UNALIGNED resources.arsc: STORED @0x3e4 ALIGNED
|
||||
zipalign -c -v 4 -> rc=1 zipalign -c -v 4 -> rc=0
|
||||
```
|
||||
|
||||
### 3.5 Evidence, a real 435-entry package, and an independent re-check
|
||||
|
||||
Same sample, same patch, old writer versus new writer versus the input:
|
||||
|
||||
```
|
||||
extra differ 205/435 (nothing but offset)
|
||||
eattr differ 435/435
|
||||
iattr, csys, ver_made differ 272/435
|
||||
flags differ 164/435
|
||||
untouched entries with changed CRC/size : 132 untouched entries with changed CRC/size : 0
|
||||
file size 4,293,620 -> 3,897,949 file size 4,293,620 -> 4,289,340
|
||||
resources.arsc: @0x36f931 UNALIGNED resources.arsc: @0x3cec08 ALIGNED
|
||||
```
|
||||
|
||||
The old writer also *shrinks* the archive by ~396 KB: re-compressing entries that were already
|
||||
compressed changes the container, not the payload.
|
||||
|
||||
This pass re-checked the decisive claim with the Android build-tools' own verifier — a tool in
|
||||
neither teammate's write scope — on the artifacts the other teammate produced:
|
||||
|
||||
```
|
||||
$ E:\tools\android-14\zipalign.exe -c -v 4 <file>
|
||||
plain.old.apk rc=1 Verification FAILED plain.new.apk rc=0 Verification succesful
|
||||
arsc.old.apk rc=1 Verification FAILED arsc.new.apk rc=0 Verification succesful
|
||||
r2pay.old.apk rc=1 r2pay.new.apk rc=0
|
||||
```
|
||||
|
||||
`zipalign -c 4` checks every uncompressed entry, which is stricter than Android's install gate. The
|
||||
bare-`.so` path (no container) was re-hashed as well and shows the intended zero regression:
|
||||
|
||||
```
|
||||
0790faeaec0ed4ea8bd3e859db23840dd9d1c2635330b48c4d2e3a3f68fdd371 bare.old.so
|
||||
0790faeaec0ed4ea8bd3e859db23840dd9d1c2635330b48c4d2e3a3f68fdd371 bare.new.so
|
||||
```
|
||||
|
||||
### 3.6 The refusal gate
|
||||
|
||||
Rebuilding is declined by default when the package cannot be rebuilt safely, with the measurement
|
||||
and a pointer at the tool that owns a full resource-safe repack:
|
||||
|
||||
```
|
||||
$ python skills/apk-reverse/scripts/so_constpatch.py tests/fixtures/apk/noextract.apk \
|
||||
--entry lib/arm64-v8a/libfoo.so --replace apkhuan=android -o out.apk
|
||||
extractNativeLibs=false
|
||||
UNSAFE: AndroidManifest.xml declares android:extractNativeLibs="false": ...
|
||||
REFUSED: this package cannot be rebuilt safely by this tool (see the UNSAFE line(s) above).
|
||||
* If you want a resource-safe repack, use scripts/repack.py: ...
|
||||
* ... re-run with --unsafe-rebuild to proceed anyway.
|
||||
RESULT=refused_unsafe_rebuild exit=1 output written: no
|
||||
```
|
||||
|
||||
Refusal is exit `1`, not `3`: `3` means the tool lacks the capability, whereas here the tool is
|
||||
capable and the *package* is the reason nothing was written, so it is a negative finding about the
|
||||
target. `--unsafe-rebuild` proceeds and still re-measures the STORED/alignment gate on its own
|
||||
output. Exit codes follow the round's set — `0` success, `1` negative finding, `2` usage (including
|
||||
an equal-length violation), `3` unsupported container (zip64, a non-deflate target, not a zip),
|
||||
`4` internal error with the output deleted — and `--json` carries
|
||||
`{status, exit_code, capability, evidence[], warnings[], next_action?}` plus `result`, `audit`,
|
||||
`verify` and `patch`, with the final line `RESULT=<token>`.
|
||||
|
||||
### 3.7 What section 3 does not establish
|
||||
|
||||
- **No device, no install.** No `adb` install or cold start was run. The `[-124]` install failure is
|
||||
argued from the bytes and from `zipalign`, not observed on a device. `inferred`.
|
||||
- **No signing run.** The rebuild output is unsigned by design; `repack.py` owns signing, so no
|
||||
`apksigner verify` result exists. `unverified`.
|
||||
- **No genuine zip64 sample.** zip64 is refused (`exit 3`); the refusal path was exercised with a
|
||||
hand-built non-zip file, not with a >4 GiB archive.
|
||||
- **Page alignment (4 KB/16 KB) is out of scope.** This work guarantees 4-byte alignment, the
|
||||
install gate. 4 KB/16 KB alignment for `extractNativeLibs=false` builds is `zipalign -p`'s job and
|
||||
stays with `repack.py`.
|
||||
- `--unsafe-rebuild` cannot repair an already-compressed `resources.arsc`: the method is preserved,
|
||||
the self-check reports it as a warning, and the output stays non-compliant.
|
||||
|
||||
---
|
||||
|
||||
## 4. What this pass did not do
|
||||
|
||||
- **No `pm install` of any artifact**, and therefore no cold-start or behavioural verification of a
|
||||
rebuilt or re-signed APK. Everything in §3 is container-level plus an external alignment verifier.
|
||||
- **No signing.** On this host `repack_signed` is genuinely blocked (a JDK launcher shim, no
|
||||
build-tools on `PATH`, no signer jar), so the signed-repack path was **not** executed; the
|
||||
capability verdict is a probe result, not an observation of a failed run.
|
||||
- **No smali round-trip**, for the same reason: the eight jars are absent. `smali_roundtrip` is
|
||||
reported BLOCKED and was not exercised.
|
||||
- **`dart_aot_full` was not exercised**: no snapshot dump exists in this session, and that
|
||||
capability reports an artifact the host cannot install rather than a missing package.
|
||||
- **`check_commands.py` was not run against a benchmark target**, only against this repository. Its
|
||||
false-clean rate on a foreign documentation set is unknown.
|
||||
- **The 30 skipped invocations were not individually confirmed** to be non-scripts; they are
|
||||
classified by the reason printed for each (`-m`, `-c`, an elision, or a prose line beginning with
|
||||
`python`), and a misclassification there would be a missed check rather than a false finding.
|
||||
- **One workbench nuisance observed and not cleaned up** (not in any write scope): the tree carries
|
||||
an untracked `--help/` directory containing `exempt/benign.txt` and `planted/{appkey,device,
|
||||
endpoint,package,path}.txt`, i.e. the planted leak-fixture layout. It is not produced by
|
||||
`scan_leaks.py --help` (reproduced in a scratch directory: usage only, nothing written) and not
|
||||
by this pass's tools, which now probe in a temporary directory. The generator is
|
||||
`tools/_work/leak-precedent/gen_fixtures.py`, whose output directory is taken from an argument, so
|
||||
a probe that passed `--help` as that argument would create exactly this. It is worth removing by
|
||||
whoever runs the final commit, because the directory holds deliberately leak-shaped files.
|
||||
|
||||
## 5. Files changed by this pass
|
||||
|
||||
New: `skills/apk-reverse/scripts/capabilities.py`, `check_commands.py`.
|
||||
Rewritten: `skills/apk-reverse/scripts/doctor.py` (its documented CLI is unchanged; `--scripts` and
|
||||
`--json` still work, and the JSON keeps its old keys).
|
||||
Merged here from teammate `zip-safety`'s work (their write scope, not this one):
|
||||
`skills/apk-reverse/scripts/so_constpatch.py`, `tests/test_so_constpatch.py`,
|
||||
`tests/fixtures/make_fixture_apk.py`, `tests/fixtures/apk/{plain,arsc_aligned,noextract,services}.apk`
|
||||
and `tests/fixtures/apk/.gitignore` (the repo-wide `*.apk` ignore rule needs a `!*.apk` reverse rule
|
||||
for fixtures to be stageable; `git check-ignore -v` confirms it).
|
||||
|
||||
Neither teammate committed or pushed. Nothing in §1 or §2 was measured on a device; §3's device
|
||||
work did not happen either.
|
||||
|
||||
## 6. Index lines handed to the Lead
|
||||
|
||||
- `skills/apk-reverse/SKILL.md` (and `README.md` where the tool table lists scripts):
|
||||
**`scripts/capabilities.py`** — new. "Capability registry: the single source of truth for what
|
||||
this host can actually do. Resolves a capability's real `requires` closure (tools, modules, jar
|
||||
sets, device properties, input artifacts) into `ok | partial | blocked`, with an executable next
|
||||
step and a cost estimate that names its own strength. `--list`, `--json`; used by `doctor.py`."
|
||||
- **`check_commands.py`** (repository root, sibling of `check_repo.py` / `check_refs.py`) — new.
|
||||
"Validates every documented script invocation (`python …`) against the script's own `argparse`
|
||||
table, read with `ast` so no script is executed to learn its interface. `--fix-report` prints
|
||||
`doc:line`, the command, the unknown flag and the closest known flags. Exit 0 clean / 1 drift /
|
||||
2 usage." It is a natural fourth entry in the publish-gate list wherever those are described.
|
||||
- **`skills/apk-reverse/scripts/doctor.py`** — the existing row's wording can gain: "the script
|
||||
inventory is scanned from the directory (`registered/checked = N / N`, undecidable entries counted
|
||||
as `unknown`), and capabilities are probed rather than inferred from `java`".
|
||||
- **`skills/apk-reverse/scripts/so_constpatch.py`** — the existing row can gain the clause from
|
||||
`zip-safety`'s §7: an APK rebuild preserves each entry's zip metadata (STORED stays STORED,
|
||||
`extra`/`external_attr`/`create_system` intact, `resources.arsc` kept STORED and 4-byte aligned)
|
||||
and is **refused by default** when `extractNativeLibs="false"` or `resources.arsc` is not
|
||||
STORED + aligned — `scripts/repack.py` is the route for that package, or `--unsafe-rebuild` to
|
||||
override deliberately.
|
||||
- **`repack.py:_needs_alignment` is a live defect** (`^lib/[^/]+\.so$` cannot match
|
||||
`lib/<abi>/x.so`), reported by `zip-safety` and confirmed here by reading the predicate. Someone
|
||||
needs a write scope for `repack.py`, or it should be recorded as a known gap.
|
||||
@@ -0,0 +1,361 @@
|
||||
# EXTENSION — executable test harness, CI, and the exit-code work order
|
||||
|
||||
This file records what the regression harness in `tests/` actually does, what it measured on
|
||||
this machine, and what it does **not** cover. Strength labels follow the repository rule in
|
||||
`references/long-task-discipline.md`: `observed` means an exact command and its output are
|
||||
below, `inferred` means reasoned from a neighbouring measurement, `unverified` means nobody
|
||||
has put a command behind it here.
|
||||
|
||||
The distinction this file exists to keep honest: `tests/benchmark.md` is a **record** of routes
|
||||
run once by hand against real samples and a real device. `tests/` is a **gate** that runs on
|
||||
every commit with the standard library and pytest and nothing else. Neither replaces the
|
||||
other, and a green suite is not evidence that any route works on a hardened target.
|
||||
|
||||
---
|
||||
|
||||
## 1. How to run it, and what it printed
|
||||
|
||||
```
|
||||
$ python -m pytest -q
|
||||
596 passed, 153 skipped, 26 xfailed in 49.67s observed
|
||||
|
||||
$ python -m pytest -q -m unit
|
||||
163 passed, 612 deselected in 4.50s observed
|
||||
|
||||
$ python -m pytest -q -m cli
|
||||
342 passed, 153 skipped, 255 deselected, 25 xfailed in 38.26s observed
|
||||
|
||||
$ python -m pytest -q -m integration
|
||||
65 passed, 709 deselected, 1 xfailed in 1.33s observed
|
||||
|
||||
$ python tests/run_tests.py -q -m unit
|
||||
163 passed, 612 deselected in 5.08s observed
|
||||
```
|
||||
|
||||
`tests/run_tests.py` is the one-command entry point. It refuses to fail obscurely when pytest
|
||||
is absent — measured by putting a `pytest.py` that raises `ImportError` first on
|
||||
`PYTHONPATH`:
|
||||
|
||||
```
|
||||
$ python tests/run_tests.py
|
||||
pytest is not installed; this suite cannot run.
|
||||
install: python -m pip install pytest
|
||||
the suite is stdlib-only otherwise (no device, no network, no adb)
|
||||
exit=3 observed
|
||||
```
|
||||
|
||||
Exit 3 is the repository's "environment capability insufficient" code, so a wrapper can tell
|
||||
"no pytest here" apart from "tests failed". The suite is stdlib-only apart from pytest itself.
|
||||
CI targets Python 3.11 and 3.13; this machine measured on 3.14.0 with pytest 8.4.1.
|
||||
|
||||
---
|
||||
|
||||
## 2. Coverage
|
||||
|
||||
| Area | What runs | Strength |
|
||||
|---|---|---|
|
||||
| dex format table | `insn_units` asserted opcode by opcode against 29 named traps (payload widths, `0x32`-`0x3D` being two units, `const-string` vs `/jumbo`, the `goto` family at `0x28`-`0x2A`); `OP_UNITS` and the accessor must agree for every listed opcode; unallocated opcodes still terminate | observed |
|
||||
| dex header | magic/`file_size`/table-pointer checks against a synthesised consistent header; the verified offset table asserted by value; signature-before-checksum order, including the failure mode where a checksum taken over a zeroed signature never verifies | observed |
|
||||
| branch arithmetic | `if-test`/`if-testz`/`goto`/`goto16`/`goto32` targets and register extraction, on a synthetic buffer | observed |
|
||||
| the sample regression | `dex_patch_bytes.py` + `l1_root.spec.json` → sha256 `67ae1e58…` and byte-identical to `classes.patched.dex`; the diff window is pinned (see §6) | observed |
|
||||
| scheme-less protobuf | built-in fixture set 26/26, per-fixture expectations, decode→re-encode byte-identical for every canonical fixture, the non-canonical fixture required to *report* its mismatch, both wire-format traps | observed |
|
||||
| blob rotation search | outer encodings round-trip, raw/zlib/gzip identified by name, the rotation and prefix (`skip`) parameters recovered from a payload built with known parameters | observed |
|
||||
| leak scanner | synthesised decoys in a temporary directory are reported with the right rules and strengths; the exemption list (`com.android.*`, `com.google.*`, `org.example.*`, `<pkg>`, loopback) produces no finding; an RFC 5737 address stays `weak` and keeps the default gate green while `--fail-on any` fails it; JSON report keys and the last-line `RESULT=` token | observed |
|
||||
| CLI contract | every shipped script, four invocation shapes where they apply: `--help`, unknown flag, no arguments, all inputs pointed at a missing file | observed |
|
||||
| adb argv construction | `devsh.py`, `coldstart.Device`, `install_test.adb` build an **argv list**; `\|`, `>`, `$()` and quotes survive as literal text inside one argument; serial injection as `-s`; env-var precedence | observed |
|
||||
| screenshot validation | `coldstart.Device.screencap` judges the bytes on disk: PNG magic accepted, non-PNG rejected, a sub-8-byte body rejected, a timeout reported as a failed capture | observed |
|
||||
| launch-health verdict | `install_test.main` driven with a scripted `pidof`: stable pid + no fatal signature is the only pass; a changed pid, a dead app, or a stable pid with a `FATAL EXCEPTION` line all fail; two samples are actually taken | observed |
|
||||
| log/tombstone parsing | `coldstart.signal_counts` over a logcat fixture (9 patterns, exact counts); `native_crash.parse` signal/code/pid/uptime/frames/registers; `is_system_lib` classification; the `verdict_hint` for an arranged fault | observed |
|
||||
|
||||
---
|
||||
|
||||
## 3. Not covered — the honest gaps
|
||||
|
||||
### 3.1 Device work: excluded by design, because hardware is present
|
||||
|
||||
This harness must not touch a device, and after one measured incident that is now enforced
|
||||
rather than assumed. It has to be: `adb devices` on the machine this was written on reports an
|
||||
online device, so a naive "run every script with its arguments filled in" test drives real
|
||||
hardware.
|
||||
|
||||
```
|
||||
$ adb devices
|
||||
SSBYPJKFNVEU6PGA device observed
|
||||
```
|
||||
|
||||
**What happened.** An early version of the CLI contract test executed `coldstart.py` with its
|
||||
requirements filled in. The script found the single online device, adopted it, started
|
||||
`adb logcat -v threadtime`, wrote a 927,893-byte `logcat.txt`, captured four frames, and left
|
||||
**five orphaned `adb logcat` processes** behind when the 12 s timeout killed the parent. The
|
||||
whole mess landed in `skills/apk-reverse/scripts/`, which is another owner's write scope.
|
||||
|
||||
**What was cleaned up.** The five orphaned `adb logcat` processes were terminated, and the
|
||||
directory the script created (`skills/apk-reverse/scripts/coldstart/`, containing `logcat.txt`
|
||||
and `shots/f0*.png`) was removed. Verified: `git status` on `skills/apk-reverse/scripts` no
|
||||
longer lists it.
|
||||
|
||||
**What changed in the design.**
|
||||
|
||||
- Every child process now runs in a private temporary working directory, so a script that
|
||||
defaults an output path relative to cwd cannot write into the shipped tree. This is the
|
||||
general fix; the incident was one instance of the class.
|
||||
- `contract.DEVICE_OR_NETWORK_BOUND` — 15 scripts — is never executed with complete arguments.
|
||||
`--help` and an unknown flag are still exercised for all of them, because argparse answers
|
||||
both before any device call is reached, and that was verified for every script individually:
|
||||
`--help` returned in well under a second and an unknown flag returned 2 for each.
|
||||
|
||||
**Residual risk, stated plainly.** This is a blocklist, not a sandbox. A script not on the
|
||||
list that grows device access would be executed. The list is checked against the tree on every
|
||||
run (a script that appears without being classified becomes a new pass or a new xfail rather
|
||||
than a silent gap), but it is maintained by hand.
|
||||
|
||||
Fifteen scripts are device- or network-bound and their argument-complete behaviour is
|
||||
`unverified` here. What that means concretely: this harness cannot tell you that a repack
|
||||
installs, that an anti-debug patch survives, that a hook fires, or that a tracer reports
|
||||
blocks. Those live in `tests/benchmark.md`, and the `unverified` marks there are unchanged by
|
||||
anything in this directory.
|
||||
|
||||
### 3.2 CI has never executed: `unverified`
|
||||
|
||||
`.github/workflows/ci.yml` exists and is syntactically valid — verified locally:
|
||||
|
||||
```
|
||||
$ python -c "import yaml,io; yaml.safe_load(io.open('.github/workflows/ci.yml',encoding='utf-8'))"
|
||||
ci.yml: valid YAML observed
|
||||
```
|
||||
|
||||
and every command it runs was executed by hand on this machine:
|
||||
|
||||
```
|
||||
$ python check_refs.py exit 0 observed
|
||||
$ python check_budget.py exit 0 ("0 failure(s), 0 warning(s), 3 note(s)") observed
|
||||
$ python check_repo.py exit 0 (with PYTHONUTF8=1; see §5.1) observed
|
||||
$ python -m pytest -q exit 0 ("596 passed, 153 skipped, 26 xfailed") observed
|
||||
```
|
||||
|
||||
It is still `unverified` as a workflow, and the blocker is specific: there is no GitHub Actions
|
||||
runner reachable from this machine, the workflow has not been pushed (this work item is
|
||||
forbidden from committing), and no local runner (`act`) is installed. What cannot be claimed
|
||||
until the first push: that `actions/setup-python` resolves both matrix versions, that the gates
|
||||
behave identically under a Linux UTF-8 locale, and that the suite's timing stays inside any
|
||||
runner limit. The last two matter because the gates behave *differently* on this Windows host —
|
||||
see §5.1 — and because the CLI suite spawns roughly 200 subprocesses.
|
||||
|
||||
A remote does exist (`origin` → `github.com/newliver666/apk-reverse`), so the workflow will run
|
||||
on the first push; nobody has observed it do so.
|
||||
|
||||
### 3.3 Agent behaviour: `unverified`, and deliberately out of scope
|
||||
|
||||
`skills/apk-reverse/evals/evals.json` is a skeleton: three cases with expected outputs and
|
||||
grading assertions, and an explicit statement **in the file** that nothing in it has ever been
|
||||
executed. No with-skill arm and no without-skill arm has been run. Running them means running
|
||||
the agent twice per case and grading transcripts, which was excluded from this pass by decision
|
||||
(`REBUILD-BRIEF.md` §2, the do-not-do list). The file carries its own run procedure.
|
||||
|
||||
### 3.4 The 153 skips, by category
|
||||
|
||||
| Count | Reason | Reading |
|
||||
|---|---|---|
|
||||
| 60 | device/network bound, not executed | 15 scripts × 4 scenarios. Deliberate; see §3.1. |
|
||||
| 40 | requires no argument, so the contract does not apply | 10 scripts × 4 scenarios. Requiring exit 2 from `doctor.py` or `scan_leaks.py` with no arguments would invent a rule the contract does not state. |
|
||||
| 34 | does not advertise `--json` | the 34 of 46 scripts with no `--json` flag. That is the §1.6 problem, not a harness gap. |
|
||||
| 11 | `--json` advertised but the path needs a device or a sample | includes `capabilities.py`, added during this pass. The shape is not asserted because the run that would reach it needs a target. |
|
||||
| 8 | hand-written usage, argv not derivable | `dexutil.py` and `smtool.py`, 4 scenarios each. Their usage line is prose, so the missing-input argv is not machine-derivable. `devsh.py` and `patch_smali.py` share the trait but are already covered by row 1. |
|
||||
|
||||
Total 153, which is why the headline is "passed, skipped, xfailed" and not "passed".
|
||||
|
||||
---
|
||||
|
||||
## 4. The exit-code work order — 26 xfails
|
||||
|
||||
These are not flaky tests. Each is a script on the tree that does not meet the repository
|
||||
contract in `REBUILD-BRIEF.md` §4 (`0` success, `1` target result failed, `2` bad
|
||||
arguments/input, `3` environment capability missing, `4` internal error). The xfail reason
|
||||
carries the measured value, so `pytest -q -ra` prints the work order at any time.
|
||||
|
||||
**A. `--help` does not exit 0 — 4 scripts.** A hand-written parser rather than argparse, so
|
||||
`--help` falls through to "unknown argument". None of them prints a traceback, so this is a
|
||||
contract gap and not a crash.
|
||||
|
||||
`devsh.py`, `dexutil.py`, `patch_smali.py`, `smtool.py` — measured `2`, expected `0`.
|
||||
|
||||
**B. A missing input file exits 1 instead of 2 — 13 scripts.** The distinction matters to a
|
||||
calling agent: 1 means "your target failed", 2 means "your invocation was wrong". Reporting a
|
||||
typo in a path as a target failure sends the next agent to debug the sample.
|
||||
|
||||
`apk_diff.py`, `dart_disasm.py`, `dart_pool_strings.py`, `dex_classdiff.py`,
|
||||
`dex_dump_validate.py`, `dex_find_insn.py`, `dex_mem_scan.py`, `dex_strpatch.py`,
|
||||
`elf_plt.py`, `native_crash.py`, `patch_smali.py`, `stalker_report.py`, `svc_scan.py` —
|
||||
measured `1`, expected `2`.
|
||||
|
||||
**C. An uncaught exception instead of a clean report — 7 scripts.** The user-visible
|
||||
difference: a traceback instead of a sentence.
|
||||
|
||||
`dart_pool_strings.py`, `dex_classdiff.py`, `dex_find_insn.py`, `dex_mem_scan.py`,
|
||||
`dex_strpatch.py`, `patch_smali.py`, `stalker_report.py` — all `FileNotFoundError` (on Windows,
|
||||
`dex_mem_scan.py` reports `WinError 3`).
|
||||
|
||||
**D. `--json` without a `status` field — 1 script.**
|
||||
|
||||
`preflight.py --json` returns top-level keys `rows` and `worst`, with no `status`. Per §4 the
|
||||
document must carry `status` (and ideally `exit_code`, `capability`, `evidence`, `warnings`,
|
||||
`next_action`). For contrast, `doctor.py --json` was measured during this pass and *does* carry
|
||||
`status`, and now exits 3 rather than 0 when a capability is missing — the unified convention
|
||||
is arriving script by script.
|
||||
|
||||
**E. `su` quoting — 1 item, outside the exit-code list.** `devsh.su` wraps in double quotes and
|
||||
escapes only `"`, so `;`, `$()`, backticks and `|` from a `--pkg`-style value still reach the
|
||||
device shell. `coldstart.Device.shell` uses the POSIX single-quote escape and does neutralise
|
||||
them. Two helpers, two strategies — the `device_shell()` unification item. The contrast is
|
||||
asserted in `tests/integration/test_adb_command_construction.py::TestDevshSuQuotingGap`.
|
||||
|
||||
Nothing in this list was fixed by this work item: every one of those scripts belongs to another
|
||||
owner, and a test that quietly repaired its subject would stop being a test.
|
||||
|
||||
---
|
||||
|
||||
## 5. Defects found and registered, not fixed
|
||||
|
||||
### 5.1 `check_repo.py` misreports a script as crashing under a non-UTF-8 console (`observed`)
|
||||
|
||||
```
|
||||
$ python check_repo.py
|
||||
- E:\apk-reverse\skills\apk-reverse\scripts\dex_strpatch.py --help crashed:
|
||||
== result: 1 problem(s) ==
|
||||
exit=1
|
||||
```
|
||||
|
||||
with, above it, `UnicodeDecodeError: 'gbk' codec can't decode byte 0x94 in position 63` raised
|
||||
inside `subprocess`'s reader thread. The gate spawns each script with `text=True` and no
|
||||
explicit encoding, so on this host the child's output is decoded with GBK. One script's
|
||||
`--help` text is not GBK-decodable, and the gate attributes the failure to that script. The
|
||||
script itself is fine:
|
||||
|
||||
```
|
||||
$ python skills/apk-reverse/scripts/dex_strpatch.py --help ; echo $?
|
||||
0 observed
|
||||
```
|
||||
|
||||
Pinning the codec fixes it:
|
||||
|
||||
```
|
||||
$ PYTHONUTF8=1 python check_repo.py
|
||||
== result: 0 problem(s) ==
|
||||
exit=0 observed
|
||||
```
|
||||
|
||||
`PYTHONUTF8: "1"` is therefore set in `ci.yml`. Two caveats: CI runs on Linux, where the default
|
||||
codec is already UTF-8, so this is a portability fix for anyone running the gates on Windows
|
||||
rather than a fix for the CI run; and `check_repo.py` is in the Lead's write scope, so the
|
||||
underlying `text=True` call is not changed here.
|
||||
|
||||
### 5.2 `native_crash.py` overwrites `regs['pc']` with a backtrace frame (`observed`)
|
||||
|
||||
Register extraction scans every line of a crash block with one permissive pattern, so a
|
||||
`#NN pc <hex>` backtrace line matches too and the last one wins. On the fixture, the register
|
||||
dump's own `pc 00000000007a111128cc` is not what `regs['pc']` holds; `0x55488` — the last
|
||||
frame's pc — is. The verdict is unaffected (the arranged-fault hint keys off the fault address
|
||||
and the registers holding it, and neither is `pc`), but a reader who trusts `regs['pc']` as the
|
||||
faulting instruction pointer is misled. Pinned in
|
||||
`tests/integration/test_no_device_flows.py::TestTombstoneParsing`.
|
||||
|
||||
### 5.3 `coldstart.installed_facts` `versionCode` swallows the rest of the line (`observed`)
|
||||
|
||||
The extraction is `r'\s*(versionCode|...)=(.*)'`, so on a real `dumpsys package` line —
|
||||
`versionCode=42 minSdk=24 targetSdk=34` — the value is the whole remainder, not `42`. It still
|
||||
works as the before/after fingerprint the caller uses it for, but the field name overstates
|
||||
what it holds. Pinned in `tests/integration/test_adb_command_construction.py`.
|
||||
|
||||
### 5.4 The leak scanner reports one `ip:port` twice (`observed`)
|
||||
|
||||
`192.0.2.10:8080` satisfies both `plain_addr` (`ip:port`) and `plain_addr_ipliteral` (bare
|
||||
IPv4), because the literal rule's trailing `(?![\w.])` is not violated by a colon. Both hits
|
||||
are `weak`, so the default gate stays green; the effect is report volume, not a wrong verdict.
|
||||
Pinned, with the reasoning, in `tests/unit/test_scan_leaks.py`.
|
||||
|
||||
### 5.5 `usb_net_proxy.py` does something with no arguments (`observed`)
|
||||
|
||||
With no arguments it neither prints usage nor exits: it ran past both a 12 s and a 20 s timeout
|
||||
during probing. Every other script either requires arguments or prints its own documentation.
|
||||
Not diagnosed further here — the fix belongs to whoever owns that script. It is classified as
|
||||
device/network bound, so the suite does not execute it.
|
||||
|
||||
### 5.6 Stray artifacts in the tree that this work item did not create (`observed`)
|
||||
|
||||
Found while checking its own side effects, left in place because they are not this work item's
|
||||
to delete and may belong to a concurrent one:
|
||||
|
||||
- `--help/` at the repository root, containing `planted/{package,device,appkey,endpoint,path,token}.txt`
|
||||
and `exempt/benign.txt`. The names say "leak-scanner decoy set", so some scanner test wrote
|
||||
its fixtures into a directory literally named `--help` — most likely an argument-parsing bug
|
||||
in that test, not a scan result.
|
||||
- `skills/apk-reverse/scripts/example.preferences_pb` and `skills/apk-reverse/scripts/snapshots/`
|
||||
(containing `snap.png`, 3.97 MB, and `_snap_ui.xml`). Timestamps 01:17 and 01:18, unchanged
|
||||
across this work item's later runs, so they are not its output.
|
||||
|
||||
---
|
||||
|
||||
## 6. The established regression marker
|
||||
|
||||
The red line recorded in `REBUILD-BRIEF.md` §2 is reproduced by the suite rather than
|
||||
re-invented:
|
||||
|
||||
```
|
||||
$ python skills/apk-reverse/scripts/dex_patch_bytes.py \
|
||||
tools/_work/bench/l1l3/dex/classes.dex \
|
||||
--spec tools/_work/bench/l1l3/l1_root.spec.json \
|
||||
-o <tmp>/classes.patched.dex
|
||||
== header checksum 0x7fb7d9fa -> 0x86e1d928
|
||||
signature b7fafe72cb521450 -> eb632328fd1ba03c
|
||||
self-verify: checksum_ok=True signature_ok=True
|
||||
== wrote <tmp>/classes.patched.dex (5528 bytes, delta 0)
|
||||
|
||||
$ sha256sum <tmp>/classes.patched.dex
|
||||
67ae1e58e7a44777e5ac8f9f823ab21e55e56c6122d2d64c3627232d902a2bba observed
|
||||
```
|
||||
|
||||
Identical to the stored `tools/_work/bench/l1l3/dex/classes.patched.dex`, byte for byte. The
|
||||
suite asserts the hash **and** where the difference is allowed to be: bytes 8..32 (checksum and
|
||||
signature, recomputed) plus the four bytes at `0x9d0`. Two of those four keep their original
|
||||
value — `39000e00` → `00000000` clears two bytes — and a recomputed checksum can keep a byte by
|
||||
coincidence, so the assertion is containment plus "these must have moved", not equality of the
|
||||
diff set. That is the honest form: an equality assertion would fail on a correct build.
|
||||
|
||||
The test copies the bench input into `tmp_path` before patching, so a test run cannot overwrite
|
||||
the stored expectation.
|
||||
|
||||
`tools/_work/bench/**` is git-ignored, so on a fresh checkout and in CI this test **skips** with
|
||||
the printed reason instead of turning red. A regression marker that fails because a sample is
|
||||
absent is a marker people learn to ignore.
|
||||
|
||||
---
|
||||
|
||||
## 7. Files added by this work item
|
||||
|
||||
```
|
||||
pytest.ini repository root
|
||||
tests/README.md how to run it, and the ground rules
|
||||
tests/run_tests.py entry point; exits 3 without pytest
|
||||
tests/conftest.py fixtures, subprocess runner, isolated cwd
|
||||
tests/unit/test_dexutil.py
|
||||
tests/unit/test_protobuf_decode_raw.py
|
||||
tests/unit/test_blob_decode.py
|
||||
tests/unit/test_scan_leaks.py
|
||||
tests/unit/test_dex_patch_bytes_regression.py
|
||||
tests/cli/contract.py usage parsing and the contract constants
|
||||
tests/cli/test_cli_contract.py
|
||||
tests/integration/test_adb_command_construction.py
|
||||
tests/integration/test_no_device_flows.py
|
||||
tests/fixtures/tombstone_sample.txt synthetic, 1.3 KB
|
||||
tests/fixtures/logcat_sample.txt synthetic, 1.0 KB
|
||||
.github/workflows/ci.yml
|
||||
skills/apk-reverse/evals/evals.json skeleton, never executed
|
||||
```
|
||||
|
||||
Not touched: `skills/apk-reverse/scripts/**`, `tests/test_so_constpatch.py`,
|
||||
`tests/fixtures/apk/**`, `SKILL.md`, `README.md`, the root `check_*.py` scripts. No commit, no
|
||||
push. The only write this work item made inside the shipped tree was its own accident (§3.1),
|
||||
and it was reverted.
|
||||
|
||||
Fixture budget: two text fixtures, 2.3 KB total, well under the 32 KB per-file limit. The leak
|
||||
decoys are synthesised at run time and are not committed.
|
||||
@@ -68,6 +68,8 @@ those are called out in the matrix rather than buried here.
|
||||
| `EXTENSION-vmp-diff.md` | The known-plaintext differential: a 218-of-224-opcode coverage fixture, a closed-loop check that recovered 218/218 mappings with zero fabrications, and the one link that cannot be automated |
|
||||
| `EXTENSION-kernel-weapons.md` | The generated KernelSU/APatch scaffold and its three kernel-side templates, all shipped unbuilt, plus the structural correction that a userspace module cannot change a syscall return value |
|
||||
| `EXTENSION-split-apk.md` | Two real split sets through both branches: unified re-signing, the merge that is refused by design, the merge that succeeds, and the signer chain this host actually needs |
|
||||
| `EXTENSION-reconstruction.md` | The reconstruction pass: the doctor capability closure (a JDK-only PATH used to report "can re-sign", now `BLOCKED` with a next action), the zip rebuild that stops dropping per-entry metadata (`zipalign -c -v 4` fails the old output and passes the new), and `check_commands.py` catching a documented flag that does not exist |
|
||||
| `EXTENSION-test-harness.md` | The executable half of the evidence: what `tests/` asserts and what it measured, the `dex_patch_bytes` regression reproduced byte-for-byte, the 153 skips and 26 xfails that form the exit-code work order, the device-bound exclusion forced by an incident where the suite drove a real phone, and CI written but never run |
|
||||
| `EXTENSION-protobuf-raw.md` | A schema-free decoder cross-checked against the official runtime and a real DataStore container, with the packed-boundary and proto3-zero ambiguities reproduced on real bytes |
|
||||
|
||||
## Absorption-pass record
|
||||
|
||||
Reference in New Issue
Block a user