Skip to content

fix(update): observe isolated Mac hop accurately - #79

Merged
andersalm merged 3 commits into
feat/0.7.1-modelsfrom
feat/0.7.1-update-hop
Sep 30, 2026
Merged

andersalm merged 3 commits into
feat/0.7.1-modelsfrom
feat/0.7.1-update-hop

Conversation

@andersalm

@andersalm andersalm commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Part of #106. This PR corrects the observer for the approved isolated update-hop fixture.

The observer binds the operator gateway's bootstrap DID to the fixture identity read through the local CLI before gateway startup. It checks that binding before peer admission or Apply. Update-attempt evidence uses the expected new binary hash or the exact expected-version install line; cache-stage evidence uses complete updater lines, and response-loss evidence requires host exit 75.

Raw components.json hashes remain separate from semantic JSON comparison. The earlier Mac fixture runs showed that the updater rewrites those bytes and that operator self-update can lose its response after host exit. The product manifest-byte correction belongs to #107. Those retained runs remain evidence for their original candidate.

Validation: seven mocked/pure observer tests pass, including spoofed target rejection before admission, missing identity, unrelated stage text, wrong or missing hashes, timeout without supersession exit, and raw manifest hash failure despite matching JSON. The spoofed-target test fails on the prior observer and passes with this correction. CI and local pre-push now run the suite. The four AGENTS source gates, YAML parsing, shell syntax, and independent review pass. Exact candidate and proof are recorded in #106.

The correction uses observer tests and source checks. The coordinator will merge after the corrected candidate passes the required CI checks and review gates.

@andersalm andersalm mentioned this pull request Sep 29, 2026
5 tasks done
@andersalm
andersalm requested a review from irzhywau September 29, 2026 16:40
@andersalm
andersalm removed the request for review from irzhywau September 29, 2026 19:33
@andersalm
andersalm marked this pull request as ready for review September 29, 2026 19:38
@andersalm
andersalm requested a review from irzhywau September 29, 2026 20:51
@andersalm

Copy link
Copy Markdown
Contributor Author

@irzhywau ready for your review. Small change to the update test script (scripts/update-hop-compare.py). Focus: does the comparison really prove what #106 records about the current update behaviour, without hiding a difference? Part of #106 (UP-00).

@irzhywau

Copy link
Copy Markdown
Collaborator

Security review — #79 fix(update): observe isolated Mac hop accurately

@andersalm — security review of head a38015e4 against feat/0.7.1-models. Review only; no changes were made to the branch.

Verdict: approve from a security standpoint. The PR only changes scripts/update-hop-compare.py, an operator-approved test observer. No product code, no shipped path. Findings are Low / Info. F3 is a product observation the PR surfaces and should be tracked in the UP-01 handoff.

Severity scale: Critical exploitable now with direct impact · High realistic path to code execution or integrity loss · Medium needs an extra condition · Low hardening / fixture trust · Info no action.


F1 — Low · Operator target identity is taken from an unauthenticated HTTP response and never checked

Where: scripts/update-hop-compare.py:320-326 (info()), used at 369-372; start() at 340-350.

Threat. For the operator path, the target's did and connect_ticket now come from GET http://127.0.0.1:<port>/.well-known/elastos/carrier-bootstrap.json?role=publisher, and the controller then admits that peer (node peer add --did … --ticket …). The publisher value is anchored: line 364 compares it with the signed fixture's publisher_dids. The target value is not compared with anything.

start() probes the port with bind() then closes it before spawning the gateway, which leaves a time-of-check/time-of-use window. Readiness only checks /healthz plus the lock file of the fixture's own data dir, and a squatter can answer /healthz too. A local process that grabs the port in that window can make the controller dial and trust a DID it controls, and then receive the update.apply exchange.

Impact is limited to a local test fixture on the operator's machine, hence Low. But the observer is used as release evidence (G6), so a spoofed target would falsify the proof.

Fix:

  • After start(role) returns, compare the bootstrap DID with the target's own identity read through the CLI, which touches the fixture data dir directly and not the network:
    local = command(role, ["node", "info", "--json"], role + "-info")  # existing path used for controller
    need(read(output / (role + "-info.log"))["did"] == target["did"], "operator target identity differs from its gateway bootstrap")
    If node info cannot run while the gateway holds the lock, which was likely the reason for the switch, read the DID from the fixture's identity file instead, or record it during fixture preparation and pin it in the fixture JSON as done for publisher_dids.
  • Optionally, confirm the listener belongs to the spawned proc.pid. On macOS: lsof -nP -iTCP:<port> -sTCP:LISTEN -t equals the child pid.

F2 — Low · Evidence classification relies on loose log-string heuristics

Where: update-hop-compare.py:388 ("attempted": "Installing " in apply_text or …), 148-150 (operator_response_lost), 390 (cache_stage_observed).

Threat. attempted becomes true when the substring "Installing " appears anywhere in the host or command logs, including unrelated capsule installs. operator_response_lost accepts any connection lost … timed out, including a genuine network timeout. Both feed the G6 verdict (263-270). A mis-classified run could record "G6 failed as reproduced" or "passed" on the wrong evidence. That undermines the proof, not the product.

Fix: anchor on the updater's exact stage lines. For example, match the Installing <version> line with the expected new version:

re.search(r"^.*Installing .*" + re.escape(config["new"]["version"]), apply_text, re.M)

and keep the binary-hash change as the primary signal. For operator_response_lost, also require after["host_exit"] == 75 (already in reproduced, but not in the stored observation), so the stored flag means what its name says.


F3 — Info (product, track in UP-01) · Installed components.json cannot be re-verified against the signed artifact by hash

Where (observed, not changed here): the updater's install step rewrites components.json. The installed bytes differ from the signed release artifact while the JSON content is the same. The PR adds json_digest (62-63) and components_content (251-253) to compare parsed content instead.

Threat (product): after install, nothing on disk can be checked against the release signature byte-for-byte. Any later check of "is my installed component manifest the one the publisher signed?" has to trust a re-serialisation. That widens the parser-differential surface: duplicate keys (Python keeps the last one; Rust structs reject them), number normalisation, and so on. The observer change itself is fine, because it still reports the raw byte check components_sha256 separately. Do not let the semantic check replace the byte check in any product verification path.

Recommended product fix (for the updater owner, not this PR): install the verified artifact bytes verbatim, i.e. rename the downloaded and verified file into place, instead of deserialising and re-serialising it. Keep any local state in a separate file.


Checked and fine

  • Output directory created 0o700 and must not already exist; fixture root must avoid tmp / private / target; an exact approval reference is required.
  • Every HTTP call disables proxies (ProxyHandler({})) and binds to 127.0.0.1.
  • Publisher identity is pinned to the signed fixture (364); release artifact hashes are checked against the signed release (inspect).
  • The switch from serve to gateway --addr 127.0.0.1:<port> keeps the fixture on loopback.

@irzhywau irzhywau left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please see comment above (non-blocking)

@andersalm

Copy link
Copy Markdown
Contributor Author

@irzhywau thank you. F1 and F2 get fixed in this PR before it merges: the operator target DID is checked against the fixture's own identity (or a pinned value), and the evidence flags match the updater's exact stage lines with the expected version and exit code. F3 is a real product point: the updater must install the verified artifact bytes as they are, and a content comparison must never replace the byte check. It is recorded as a requirement in UP-01 (#107).

Read the operator fixture DID before starting its gateway and verify the bootstrap identity before peer admission. Anchor update and cache stages to exact lines, require the expected binary hash, and bind response-loss classification to host exit 75.

Validation: seven mocked/pure observer tests, four AGENTS source gates, YAML parsing and shell syntax. The spoofed-target regression fails on the prior observer and passes here.
@andersalm

Copy link
Copy Markdown
Contributor Author

@irzhywau F1 and F2 are addressed in 76994bd4.
F1 now reads the operator's DID from its local fixture CLI before starting the gateway, then checks the bootstrap DID before either peer admission or Apply.
F2 uses the exact old → expected-new version line or the expected new binary hash for an attempt, anchors cache-stage lines, and requires recorded host exit 75 for the response-loss flag. The raw component hash check stays separate from JSON-content comparison.
All seven focused tests and the four source gates pass. Independent review reran the tests and found no remaining issue in this correction. The spoofed-target regression fails on the prior observer and passes here; the tests are wired into CI.
F3 is an explicit UP-01 requirement in #107. Candidate proof records the scope; the prior real-run receipts remain bound to their original candidate.

@andersalm
andersalm merged commit ea670e2 into feat/0.7.1-models Sep 30, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants