The research landed without its index, and the review's three code findings are still open by claude[bot] · Pull Request #218 · hleserg/Attadipa

claude · 2026-08-25T00:08:14Z

Fixes #142

Research only. No Attadipa code changed and none should have — this
repository links no MeshCore and has no radio stack. Documentation, plus the
harness that was already merged.

Problem

Three things, and only the first was the plan.

#160 landed in two pieces and main currently holds the smaller one.
a7624b0 merged it as 14 of its 19 files — the report and the whole
docs/research/meshcore-parser-bounds/ harness — and left STATUS.md,
TASKS.md, VERIFIED_FACTS.md, OPEN_QUESTIONS.md and REUSE_LEDGER.md
behind. Nothing that landed is wrong. What is missing is the index around it:
CLAUDE.md sends the next agent to STATUS.md for "where things actually
are", and today it says this research does not exist, while the document's own
References line sends a reader to OPEN_QUESTIONS M15–M17 — which on main
belong to the frame-capacity work, not to this. A green partial merge looks
exactly like a whole one from the commit list.

The independent review of #160 left one blocking finding and two real code
findings, and the blocking one got worse while the branch sat: it stamped this
work T-127, which main now uses twice (TASKS.md:2935 the anchor check,
:3406 the frame capacity), and STATUS.md called it T-051, which is the
not-started P1 MIA-M10Q GNSS research. The M15–M18 open questions
collided the same way. Findings 3 and 4 are the harness quietly measuring the
wrong revision — described below, because they are the ones with teeth.

The owner posted an upstream delta on 2026-08-24 — Meshtastic
firmware#11573 — asking for
the invariant rather than the code.

Solution

The identifiers

T-173 and M20–M23, the first free numbers, with a note in each file
saying what they were filed as. CI could not have caught the original:
check_docs.py checks duplicate IDs within TASKS.md, so an ID consistent
there and wrong in STATUS.md passes every check. Also merged the two
VERIFIED_FACTS.md entries that both said our MeshCore pin is upstream's tip —
written hours apart by two research runs from different API calls.

The harness could not check the revision it was pointed at

Both of these are enrolled as revision checks by the T-013 entry condition,
which is exactly when nobody will re-read the harness.

build-extras.sh hardcoded tree-base. ./build.sh cand <sha> followed
by ./build-extras.sh built decrypt_bounds from whatever base last held
and printed only built build/decrypt_bounds. A candidate where upstream had
fixed Utils::decrypt would be recorded as still broken, silently. It now
takes the tag, and build.sh writes the resolved SHA to
build/tree-<tag>/.revision so both binaries can print what they speak for.
path_arith printed the same answer for every revision, including one
where upstream had fixed Mesh.cpp:172 — it hand-copied MAX_PACKET_PAYLOAD,
MAX_PATH_SIZE and isValidPathLen and compiled nothing from the tree. The
constants and the validator now come from the built tree (Packet.cpp linked,
mesh::Packet::isValidPathLen called). The eight lines of index arithmetic
genuinely cannot run on a host — they sit behind MACThenDecrypt, so behind
AES, SHA-256 and ed25519 — so rather than pretend, the PAYLOAD_TYPE_PATH
branch is extracted between two anchors, whitespace-normalised, hashed and
compared against the pin. A revision that has touched those lines is refused
with exit 65 and told to re-read P3 by hand.

#3269's head is such a revision, so this has a one-command demonstration
rather than an argument:
```
$ ./build-extras.sh pr3269
The PAYLOAD_TYPE_PATH branch in src/Mesh.cpp has changed at 5ebf8ef9…
    expected 5eee273c…
    found    e97ef164…
$ echo $?
65
```

P4's publication question is narrower than it read, and it is the owner's.
The document said reporting upstream was their call — right — but left the whole
disclosure decision sounding open. It is not: this repository is public, and
opening #160 published the affected range, the faulting line and a one-command
reproduction. What remains is an ordering decision, and §5 now says so.

Minors from the same review: §3 P3's results fence is the tool's real output
instead of a paraphrase wearing its clothes; mmap/mprotect are checked in
decrypt_bounds.cpp, because an unchecked one faults in a way that looks like
the finding and is not it; References said M15–M17 where four were filed.

§8 — a second ecosystem, verified rather than repeated

Meshtastic firmware#11573, merged ac330e6a on 2026-08-23. Every claim in
the owner's summary was re-read from the merged diff and held; the metadata
came from the API rather than the summary. It replaces
assert(p->encrypted.size <= sizeof(radioBuffer.payload)) with an executable
rejection that releases the packet and returns 0 before the memcpy, makes
startSend unwind on that zero, restores the radio's home configuration even
when there was no packet, releases the beacon packet on ERRNO_SHOULD_RELEASE,
and adds a test asserting the rejection — return value, no half-started
transmit, and the pool slot reusable. That third assertion is the one worth
copying: "it rejected" and "it rejected without losing the buffer" are different
claims.

GPL-3.0, and not in any release — compare/master...ac330e6a answers
diverged, and the newest release predates it by two months, so the shipping
firmware still has the assertion. Nothing is copied and nothing downstream is
called fixed. What changes is the weight of P4: an unchecked wire length reaching
a fixed destination is now a two-instance pattern across unrelated firmwares
rather than one project's defect. The invariant and the test shape are written in
our own words as entry conditions on T-013 and T-050.

Deliberately not claimed: whether that assert() was compiled out in
shipping builds. NDEBUG is in three of that repository's files and in none of
its build flags, and the toolchain's own definition was not traced. The narrower
claim stands without it.

Upstream re-checked, 2026-08-24

All five pull request states, heads and bases unchanged; the three open ones
last touched 2026-08-22. main has moved two commits ahead of the pin and
both are docs/faq.md, so the pin is no longer the literal tip and is still
upstream's newest code and newest release. Recorded as two separate
sentences, because only one of them ages.

Upstream and reuse

meshcore-dev/MeshCore, MIT, pinned d929643…. Nothing vendored; all
three pull requests are still open, unreleased, and two do not close their own
findings.
meshtastic/firmware, GPL-3.0, ac330e6a — read-only evidence, never a
source of code. The ledger row says so and explains why a GPL row sits in a
table of otherwise-takeable candidates.

Tests

cmake -S . -B build && cmake --build build && ctest --test-dir build --output-on-failure — 33/33 pass. This branch touches no code, so that is
"nothing regressed", not evidence for any claim here. (python3-pil must be
installed or the build registers a deliberately failing placeholder.)
python3 tools/docs/check_docs.py — clean. It caught two citations my inserts
pushed off their lines; both are re-pointed at the facts they name.
shellcheck — clean on all three scripts.
The harness was re-run from the committed copy, four trees, after the
changes: §4 reproduces exactly. #3269's ten rows are byte-identical to
base, which is expected and now measured — the report said its diff was a log
line, which was a reading of a diff rather than a run.
./build-extras.sh pr3269 refuses with exit 65, as above.
The six add/add merge conflicts were resolved by sha256sum per file against
0ac1615, confirming main's copy of each is byte-identical to what the fixes
were applied to — a strict superset, not a discarded change.

Hardware

NOT EXECUTED — HARDWARE REQUIRED. Nothing ran on a radio, a node or any
board, on either half. No host sanitizer result may be presented as radio or HIL
validation, and the four boards Meshtastic's author lists were not touched here.

Risks

A GPL row in the reuse ledger could be read as a candidate. It is not, and
the row says why in its own text rather than relying on the licence column.
The fingerprint check will fire on a legitimate upstream fix, which is the
point — but it means a future agent meets a red exit 65 and has to read the
message rather than assume the tool is broken. Said in the message itself, in
the README and in the T-013 entry condition.
P4 remains unreported upstream while the owner has not decided, and the
reproduction is now permanently in main.

Remaining blockers

None blocking. M20–M23 in OPEN_QUESTIONS.md are what this could not
establish, and none of them blocks anything while Attadipa compiles no MeshCore.
needs-owner stands for one decision only: whether upstream is told about P4
before or after the fact.

🤖 Generated with Claude Code

…ree fixes is finished Research only, from issue #142. No Attadipa code changed and none should have: this repository links no MeshCore. What changes is the evidence a future pin decision is made on, and the criteria are now written down instead of implied. Three upstream pull requests claim missing length checks in MeshCore's frame and advert parsers. The issue was cautious about comparing them to our pin because they are based on `dev`, and that caution turns out to be unnecessary: every file any of them touches is byte-identical between `d929643` and both of their bases, so their diffs apply to us unchanged and a measurement on a pull request head is a measurement of our pin plus that pull request's guards. `d929643` is also upstream's current `main` tip and its newest release, so the pin is not lagging — nothing containing these guards has shipped. The findings hold. A harness compiling four upstream translation units unmodified, with each input exactly its declared length ending flush against a PROT_NONE guard page under ASan, over-reads on nine of ten cases at named lines. The first version used a tight malloc alone and ASan does not report a read at offset 0 of a malloc(0), which silently passed two real findings; the guard page is the mechanism of record and the note is in the harness so the next person does not lose the same hour. What "out-of-bounds" is worth here is smaller than it sounds for the three the pull requests describe. At every reachable call site the backing buffer is a fixed array big enough that the read stays inside the allocation -- 256 B in Dispatcher::checkRecv, 262 B and 250 B in the two bridges, the Packet object itself for adverts -- and all nine cases end in a rejected packet. The one path where Attadipa is the party supplying the bytes, CMD_IMPORT_CONTACT, cannot reach the worst of them at all: it gates on len > 98 and Packet::readFrom reaches at most byte 70. Two things the pull requests do not say, and one nobody has said. #3269's entire diff logs the condition and then performs the read -- no break, no return -- and #3270 leaves app_data[0] unguarded for app_data_len == 0, measured on its own head, so both are incomplete against their own findings and #3271 is the same commit as #3270 rather than merely equivalent. Separately, Utils::decrypt rounds its output up to whole blocks and documents the precondition its on-air caller does not honour: 192 bytes into a 184-byte stack buffer for payload_len 181..184, reproduced against the real translation unit. That one is a write, it is in no pull request, and both it and the PATH extra_len underflow sit behind a two-byte MAC, which is not the authentication gate it looks like. Reporting that upstream is the owner's call and deliberately not done here. attadipa_link's own decoder was checked rather than assumed and is not analogous -- it validates the declared length before reading, behind a length check and a CRC. Nothing in link/ changed. No ADR changed either: ADR-0008 already treats the node as a peer, and an ADR edited to say what it said is churn. What could not be established is a section of its own, not a footnote: neither arithmetic finding was driven end to end, the stale bytes they read may or may not be groomable, what the eight over-written bytes hit on an ESP32-S3 is a frame layout nobody here has compiled, and a ten-case corpus demonstrates rather than searches. M15-M18. The harness is kept because the alternative to a runnable one is a table nobody can re-derive when upstream moves; it is under docs/ so no glob can sweep it into a build. Running it is now the entry condition on T-013's pin choice, and path_arith and decrypt_bounds are not in its matrix, so a green run.sh is not a clean revision. Neither README needed a counterpart edit -- nothing in either describes MeshCore revisions or this research. Tests: cmake configure, build and ctest -- 24/24 pass, unchanged by this branch, which touches no code. Harness verified from the committed copy. Nothing ran on a radio, a node or a board: NOT EXECUTED -- HARDWARE REQUIRED. Fixes #142 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…the link Correcting this branch's own first commit. The report said Dispatcher::checkRecv was the sole caller of tryParsePacket, which a grep across the tree contradicts: examples/companion_radio/MyMesh.cpp:2000 calls it too, from CMD_SEND_RAW_PACKET, on a client-supplied buffer gated only by len >= 4. That is not a detail. It is the single place in the whole review where a *client* hands bytes to a MeshCore parser, and a client is precisely what Attadipa is on the node path — so the one direction the report described as having no exposure at all turns out to have exactly one. Four bytes from any connected app make the node read four bytes of its previous command frame. The consequence is still nil, and for the same reason as everywhere else here: cmd_frame is a 177-byte member array, the parser's furthest reach is raw[5], the read stays inside the allocation and the call returns ERR_CODE_ILLEGAL_ARG. The neighbouring companion path was checked in the other direction and holds -- CMD_IMPORT_CONTACT gates on len > 98 while Packet::readFrom reaches at most byte 70, so that one cannot be triggered at all. What changes is the reachability claim, which was wrong, and what a future Attadipa client that emits raw packets has to know. Two smaller fixes found in the same pass: the three memcpys in onContactResponse are at MyMesh.cpp:722, :733 and :744, not :735, and `i` is 8 in the status and telemetry branches but 6 in the binary-response one. Found by verifying the report's own citations against the tree rather than against my notes, which is where the missing caller had been lost. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Checked every citation in the report against the tree instead of against what I had written down while reading, which is how the missing tryParsePacket caller got in. Four were off by one or two lines: the "multiple of 16" comment is src/Utils.cpp:81, the decrypt loop is :76-79, the Attadipa decoder's short-frame wait is frame_codec.cpp:147, and simple_repeater tests isValid() at MyMesh.cpp:657 rather than at the constructor a line above. Nothing a reader would have been misled by, and all four are the kind of thing that makes the next reader stop trusting the ones that matter. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The report reused M2's count from 2026-08-21 to say the commits dev is ahead by are elsewhere in the tree. The conclusion holds -- every file the pull requests touch is byte-identical -- but the number does not: measured today, dev at 9d7cee66 leads d929643 by 31, and 29 is #3267's base rather than dev's tip. Measured with git rev-list --count against a full clone. Worth saying how, because the first attempt returned 1: the clone had been made blobless and then deepened with --depth=1 fetches for the specific SHAs, which makes it shallow, and a shallow clone will answer a rev-list count with a confident wrong number rather than an error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…n twice The branch has been open long enough for `main` to move 35 commits under it, and the conflict was the smaller half of the problem. Three files conflicted — `STATUS.md`'s date line, and one additive block each in `TASKS.md` and `VERIFIED_FACTS.md` — and all three keep both sides, because both sides are real. What the merge exposed is that this work was filed under identifiers that other work now owns: - **`T-127` is taken twice on `main`** — the `#anchor` check at `TASKS.md:2935` and the companion frame capacity at `:3406` — so this would have been the third. It is **T-173**, the first free number. - **`STATUS.md` called the same work `T-051`**, which is the not-started **P1** MIA-M10Q GNSS research. That is the one that costs something: `CLAUDE.md` sends the next agent to `STATUS.md` for where things are, and they would have read a live P1 as closed while the research that was actually done had no entry under any real ID. Found by the independent review of #160, and CI could not have: `check_docs.py` checks duplicate IDs *within* `TASKS.md`, so an ID that is consistent there and wrong in `STATUS.md` passes every check. - **`M15`–`M18` collided the same way**, in `OPEN_QUESTIONS.md`: the frame-capacity research took M15–M19 on `main` while this was open. The four parser-bounds questions are **M20**–**M23**, with a note in place saying so. Also merged the two `VERIFIED_FACTS.md` entries that say our MeshCore pin is upstream's tip. They were written hours apart by two research runs from different API calls; one entry now carries both sources, and the other is a signpost rather than a deletion, because the failure mode of a duplicated fact is a reader citing whichever copy nobody updated. Two citations moved off their lines because of the inserts and are re-pointed at the facts they name (`VERIFIED_FACTS.md:264` and `:697`). `check_docs.py` clean. No Attadipa code in this commit and none in this branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…'s bounds fix Two separate things, both from #142 and neither of them production code. ## The independent review of #160, findings 2, 3 and 4 **`build-extras.sh` measured the wrong revision, silently.** `tree` was hardcoded to `tree-base`, so `./build.sh cand <sha>` followed by `./build-extras.sh` built `decrypt_bounds` from whatever `base` last held and printed only `built build/decrypt_bounds`. That is precisely the failure the T-013 entry condition exists to prevent, reintroduced inside the tool written to prevent it: a candidate where upstream fixed `Utils::decrypt` would be recorded as still broken. It now takes the tag, and `build.sh` writes the resolved SHA to `build/tree-<tag>/.revision` so both binaries can say what they speak for. **`path_arith` printed the same answer for every revision, including one where upstream had fixed the line.** It hand-copied `MAX_PACKET_PAYLOAD`, `MAX_PATH_SIZE` and `isValidPathLen`, and `build-extras.sh` compiled nothing from the tree — yet `TASKS.md`, the harness README and §4 all enrolled it as a revision check, which it could not be. The constants and the validator now come from the built tree (`Packet.cpp` is linked and `mesh::Packet::isValidPathLen` called). The eight lines of index arithmetic genuinely cannot be executed on a host — they sit behind `MACThenDecrypt`, so behind AES, SHA-256 and ed25519 — so instead of pretending, the `PAYLOAD_TYPE_PATH` branch is extracted between two anchors, whitespace-normalised, hashed and compared. A revision that has touched those lines is refused with exit 65 and told to re-read P3 by hand. `#3269`'s head is such a revision, so the check has a one-command demonstration rather than an argument: `./build-extras.sh pr3269` refuses, naming both digests. **The P4 publication question is the owner's, and it is narrower than it read.** The document said reporting upstream was their call, which is right, but left it sounding as if the disclosure decision were open. It is not: this repository is public, and opening #160 published the affected range, the faulting line and a one-command reproduction. What remains is an *ordering* decision — notify first, or accept publication-without-notice deliberately — and §5 now says so. Minors from the same review: the P3 results fence is the tool's real output instead of a paraphrase wearing its clothes; the `mmap`/`mprotect` in `decrypt_bounds.cpp` are checked, because an unchecked one faults in a way that looks like the finding and is not it; the References line said M15–M17 where four were filed. Re-run from the committed copy after the changes, all four trees: §4 reproduces exactly. `#3269`'s ten rows are byte-identical to `base`, which is the expected result and now a measured one — the report said its diff was a log line, and that was a reading of a diff rather than a run. Both say it now. Also re-checked upstream, 2026-08-24: all five pull request states, heads and bases unchanged; `main` has moved two commits ahead of the pin and both are `docs/faq.md`, so the pin is no longer the literal tip and is still upstream's newest code and newest release. The two halves are recorded separately because only one of them ages. ## The owner's upstream delta, verified rather than repeated Meshtastic `firmware#11573`, merged `ac330e6a` on 2026-08-23, replaces an `assert()` on a wire-supplied payload length with an executable rejection that releases the packet and unwinds the transmit state, and adds a unit test that asserts the rejection — return value, no half-started transmit, and the pool slot reusable. Every claim in the owner's summary was re-read from the merged diff and held; the metadata was taken from the API, not from the summary. It is **GPL-3.0** and it is **not in any release** — `master` does not contain the merge commit — so nothing is copied and nothing is called fixed downstream. What it changes for us is the weight of P4: an unchecked wire length reaching a fixed destination is now a two-instance pattern across unrelated firmwares rather than one project's defect. So the invariant and the test shape are written in our own words as entry conditions on T-013 and T-050, and the ledger carries the row with `ADAPT the invariant, IGNORE the code`. Deliberately not claimed: whether that `assert()` was compiled out in shipping builds. `NDEBUG` is in three of that repository's files and in none of its build flags, and the toolchain's own definition was not traced. The narrower claim stands without it — an `assert` is a statement about a program's own consistency, and a length that arrived from outside is not that. **NOT EXECUTED — HARDWARE REQUIRED** for everything physical, on both halves. Tests: `cmake && ctest` 33/33. `check_docs.py` clean. `shellcheck` clean on all three scripts. No Attadipa code changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

`a7624b0` merged #160 as **14 of its 19 files**: the report and the whole harness landed, and `STATUS.md`, `TASKS.md`, `VERIFIED_FACTS.md`, `OPEN_QUESTIONS.md` and `REUSE_LEDGER.md` did not. Nothing is wrong with what landed — the evidence is all there — but the index around it is missing, and `CLAUDE.md` sends the next agent to `STATUS.md` for where things are. Right now it would tell them this research does not exist, while the document's own References line sends them to `OPEN_QUESTIONS` M15–M17, which on `main` belong to the frame-capacity work. Six files conflicted as add/add, because a squash merge shares no history with the branch it squashed. `main`'s copy of each is **byte-identical** to the version this branch's fixes were applied to — checked with `sha256sum` per file against `0ac1615`, not assumed from the diffstat — so taking this branch's side is a strict superset rather than a discarded change. `STATUS.md` and `TASKS.md` auto-merged with #163 and #216. `check_docs.py` clean, `shellcheck` clean, `ctest` 33/33. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

github-actions · 2026-08-25T00:08:24Z

Could not determine the active pull-request count; do not assume capacity.

github-actions · 2026-08-25T07:11:00Z

This pull request has a merge conflict, so nothing is building it

Head 71bd214 reports mergeable: false (mergeable_state: dirty), and GET /commits/71bd214/check-runs answers total_count: 13.

That is not CI being slow. GitHub builds refs/pull/218/merge to run pull_request workflows against, and it does not run them at all when that ref cannot be built:

Workflows will not run on pull_request activity if the pull request has a merge conflict. The merge conflict must be resolved first.

Every check this repository produces on a pull request — CI, CodeQL and the review — is pull_request-triggered, and there is no pull_request_target workflow here, which is the one exception that page names. So none of them will run against this head while the conflict stands. That count is not zero, so read what is actually on this head before trusting it — a status left by a third-party app, and runs against an earlier head listed under the same branch, both look like coverage and are neither.

Resolve the conflict and push; the checks start on the next commit.

git fetch origin && git rebase origin/main   # or merge, either starts CI

Issue #142 carried agent:review, which says the work is with the reviewers. It is not — there is no verdict for them to have reached — so it is back to agent:ready and the next watchdog tick can pick it up.

Said once per head commit, not once per hour: push again while still conflicted and this appears again, because that push will not have been built either.

# Conflicts: # TASKS.md # docs/research/WAVESHARE_ARRIVAL.md

github-actions Bot and others added 7 commits August 23, 2026 16:10

claude Bot added the needs-owner Needs a human decision, a credential, or a settings change. label Aug 25, 2026

claude Bot mentioned this pull request Aug 25, 2026

Research: проверить новые MeshCore fuzz findings и границы parser trust #142

Closed

claude Bot mentioned this pull request Aug 25, 2026

The queue's width limit asked gh for a field it does not have #241

Merged

hleserg added the queue:parked Held open deliberately, with the owner's agreement. Not counted against the WIP limit. label Aug 25, 2026

claude Bot mentioned this pull request Aug 25, 2026

[P1] PR WIP backstop всегда получает unknown из-за неподдерживаемого baseRepository #239

Closed

hleserg removed the needs-owner Needs a human decision, a credential, or a settings change. label Aug 25, 2026

Merge remote-tracking branch 'origin/main' into cleanup/pr218

3f55ebe

# Conflicts: # TASKS.md # docs/research/WAVESHARE_ARRIVAL.md

hleserg removed the queue:parked Held open deliberately, with the owner's agreement. Not counted against the WIP limit. label Aug 25, 2026

hleserg merged commit 370aa22 into main Aug 25, 2026
13 checks passed

hleserg deleted the research/meshcore-parser-bounds-142 branch August 25, 2026 14:47

Navigation Menu

The research landed without its index, and the review's three code findings are still open - #218

The research landed without its index, and the review's three code findings are still open#218
hleserg merged 8 commits into
mainfrom
research/meshcore-parser-bounds-142

claude Bot commented Aug 25, 2026

Uh oh!

github-actions Bot commented Aug 25, 2026

Uh oh!

github-actions Bot commented Aug 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

claude Bot commented Aug 25, 2026

Problem

Solution

The identifiers

The harness could not check the revision it was pointed at

§8 — a second ecosystem, verified rather than repeated

Upstream re-checked, 2026-08-24

Upstream and reuse

Tests

Hardware

Risks

Remaining blockers

Uh oh!

github-actions Bot commented Aug 25, 2026

Uh oh!

github-actions Bot commented Aug 25, 2026

This pull request has a merge conflict, so nothing is building it

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant