fix: breadcrumbs audit batch (CRUMB-25, 28, 32, 35, 36, 37, 44) #46

Open

Breadway wants to merge 9 commits from fix/crumb-audit-batch into main

pull from: fix/crumb-audit-batch

merge into: Breadway:main

Breadway:main

Breadway:fix/eco-83-check-checkout-isolation

Breadway:fix/crumb-27-bootstrap-ssid

Breadway:fix/crumb-33-fail-closed-dns

Breadway:fix/crumb-26-latest-set-profile

Breadway:fix/crumb-31-trim-set-profile

Breadway:fix/crumb-34-list-effective-exit-nodes

Breadway:fix/crumb-30-match-fqdn

Breadway:fix/crumb-29-normalize-exit-nodes

Conversation 2 Commits 9 Files changed 8 +626 -45

Breadway commented 2026-10-20 15:35:16 +08:00

Owner

Copy link

Batch of breadcrumbs audit findings. Each card is one commit on this branch with a test that covers it.

CRUMB-25: detect --apply --json emitted more than one JSON object

• How found: audit of detect --apply --json, which printed the apply result and the detection result as separate JSON objects, so a consumer parsing one object failed.
• Fix: emit exactly one JSON object (f66fca8, src/app.rs).
• Test: unit coverage in src/app.rs for the single-object output.

CRUMB-28: connect did not propagate hidden=false to the saved profile

• How found: audit of connect. A network un-hidden in config kept 802-11-wireless.hidden=true in the NetworkManager profile, because the rewrite only ran for credential changes or hidden=true.
• Fix: profile_needs_update compares the saved hidden flag in both directions, and treats a missing key as false (d4e313a, src/nm.rs).
• Test: unhiding_a_network_triggers_a_profile_update_and_clears_the_flag and hidden_flag_matching_the_saved_profile_does_not_force_a_rewrite in src/nm.rs.

CRUMB-32: DNS pin left DHCP DNS on IPv6 when the profile had no ipv6 section

• How found: audit of apply_dns. The pin created the ipv4 section but only touched ipv6 if it already existed, so a profile without one kept auto DNS around the pin.
• Fix: the pin creates the ipv6 section the same way as ipv4, with DNS keys only and no ipv6 method. Unpin still only touches existing sections (4b5f017, src/nm.rs).
• Tests: apply_dns_pin_creates_ipv6_section_when_profile_has_none (fails before the fix), apply_dns_pin_v4_only_still_clears_ipv6_auto_dns_on_created_section (fails before the fix), apply_dns_unpin_does_not_invent_ipv6_section, apply_dns_unpin_without_ipv6_section_leaves_no_ipv6_when_pinned_before.

CRUMB-35: a blank password answer could wipe a network created concurrently

• How found: audit of cmd_add. existing was computed from the config loaded at startup, before the interactive prompt. The locked Config::update closure reloads the config, so a network created in the meantime was treated as new and got its password set to none.
• Fix: the existence check runs inside the locked closure against the fresh config. The keep-or-write decision is the pure function password_to_write. The prompt stays outside the lock (c3889ea, src/app.rs).
• Tests: blank_prompt_keeps_password_when_network_exists_in_fresh_config, blank_prompt_sets_no_password_for_a_new_network, non_blank_prompt_sets_password_for_existing_and_new_networks, explicit_password_always_replaces_even_when_empty.

CRUMB-36: a non-204 HTTP answer with a blocked ping was not treated as a portal

• How found: audit of the connectivity check. A captive portal that answered the HTTP probe with a non-204 status, while blocking ping, was reported as online.
• Fix: a non-204 HTTP answer together with a blocked ping is classified as a portal (a2e3e2c, src/status.rs).
• Test: unit coverage in src/status.rs for the classification.

CRUMB-44: a configured profile key with surrounding spaces could not be selected from the bus

• How found: audit of the set_profile bus command. The payload was trimmed before the config lookup, so a configured key such as " home " could never match. A whitespace-only payload also reported missing string "profile", which was misleading.
• Fix: the payload is forwarded untrimmed. On apply, an exact configured key wins, and the trimmed name is the fallback. A whitespace-only payload reports an empty-profile error (319e473, src/bread_events.rs).
• Tests: src/bread_events.rs unit tests (exact_profile_key_with_spaces_resolves_to_itself, padded_payload_falls_back_to_trimmed_key, exact_key_wins_over_trimmed_key_when_both_exist, whitespace_only_payload_reports_empty_profile_not_missing_field, absent_or_non_string_payload_reports_missing_field). The integration test set_profile_command_trims_name_before_applying is renamed to set_profile_command_falls_back_to_trimmed_name_when_applying and now asserts the new contract. The tests use config fixtures only and do not touch the bus.

CRUMB-37: two release workflows published the same tag

• How found: audit of the release path. .github/workflows/release.yml and .forgejo/workflows/release.yml both ran on v* tag pushes, and both wrote /srv/breadway-dl/breadcrumbs/<version> and ran gh release upload --clobber. Forgejo is authoritative, since GitHub auto-mirrors from origin.
• Fix: the GitHub release workflow is workflow_dispatch only (e44b642). The Forgejo release path is unchanged.
• Test: tests/release_workflows.rs, which asserts the GitHub workflow does not trigger on tag pushes and the Forgejo one still triggers on v*.
• Note: CLAUDE.md is not tracked in this repo (removed in 7a58d4a and listed in .gitignore as local-only), so there is no tracked file to rewrite. This card's CLAUDE.md part is therefore not applicable here.

Verified locally: cargo fmt --all -- --check, cargo clippy --all-targets -- -D warnings, cargo test --release.

Refs CRUMB-25 CRUMB-28 CRUMB-32 CRUMB-35 CRUMB-36 CRUMB-37 CRUMB-44

Batch of breadcrumbs audit findings. Each card is one commit on this branch with a test that covers it. ## CRUMB-25: detect --apply --json emitted more than one JSON object - How found: audit of `detect --apply --json`, which printed the apply result and the detection result as separate JSON objects, so a consumer parsing one object failed. - Fix: emit exactly one JSON object (f66fca8, src/app.rs). - Test: unit coverage in src/app.rs for the single-object output. ## CRUMB-28: connect did not propagate hidden=false to the saved profile - How found: audit of connect. A network un-hidden in config kept `802-11-wireless.hidden=true` in the NetworkManager profile, because the rewrite only ran for credential changes or hidden=true. - Fix: `profile_needs_update` compares the saved hidden flag in both directions, and treats a missing key as false (d4e313a, src/nm.rs). - Test: `unhiding_a_network_triggers_a_profile_update_and_clears_the_flag` and `hidden_flag_matching_the_saved_profile_does_not_force_a_rewrite` in src/nm.rs. ## CRUMB-32: DNS pin left DHCP DNS on IPv6 when the profile had no ipv6 section - How found: audit of `apply_dns`. The pin created the ipv4 section but only touched ipv6 if it already existed, so a profile without one kept auto DNS around the pin. - Fix: the pin creates the ipv6 section the same way as ipv4, with DNS keys only and no ipv6 `method`. Unpin still only touches existing sections (4b5f017, src/nm.rs). - Tests: `apply_dns_pin_creates_ipv6_section_when_profile_has_none` (fails before the fix), `apply_dns_pin_v4_only_still_clears_ipv6_auto_dns_on_created_section` (fails before the fix), `apply_dns_unpin_does_not_invent_ipv6_section`, `apply_dns_unpin_without_ipv6_section_leaves_no_ipv6_when_pinned_before`. ## CRUMB-35: a blank password answer could wipe a network created concurrently - How found: audit of `cmd_add`. `existing` was computed from the config loaded at startup, before the interactive prompt. The locked `Config::update` closure reloads the config, so a network created in the meantime was treated as new and got its password set to none. - Fix: the existence check runs inside the locked closure against the fresh config. The keep-or-write decision is the pure function `password_to_write`. The prompt stays outside the lock (c3889ea, src/app.rs). - Tests: `blank_prompt_keeps_password_when_network_exists_in_fresh_config`, `blank_prompt_sets_no_password_for_a_new_network`, `non_blank_prompt_sets_password_for_existing_and_new_networks`, `explicit_password_always_replaces_even_when_empty`. ## CRUMB-36: a non-204 HTTP answer with a blocked ping was not treated as a portal - How found: audit of the connectivity check. A captive portal that answered the HTTP probe with a non-204 status, while blocking ping, was reported as online. - Fix: a non-204 HTTP answer together with a blocked ping is classified as a portal (a2e3e2c, src/status.rs). - Test: unit coverage in src/status.rs for the classification. ## CRUMB-44: a configured profile key with surrounding spaces could not be selected from the bus - How found: audit of the `set_profile` bus command. The payload was trimmed before the config lookup, so a configured key such as `" home "` could never match. A whitespace-only payload also reported `missing string "profile"`, which was misleading. - Fix: the payload is forwarded untrimmed. On apply, an exact configured key wins, and the trimmed name is the fallback. A whitespace-only payload reports an empty-profile error (319e473, src/bread_events.rs). - Tests: src/bread_events.rs unit tests (`exact_profile_key_with_spaces_resolves_to_itself`, `padded_payload_falls_back_to_trimmed_key`, `exact_key_wins_over_trimmed_key_when_both_exist`, `whitespace_only_payload_reports_empty_profile_not_missing_field`, `absent_or_non_string_payload_reports_missing_field`). The integration test `set_profile_command_trims_name_before_applying` is renamed to `set_profile_command_falls_back_to_trimmed_name_when_applying` and now asserts the new contract. The tests use config fixtures only and do not touch the bus. ## CRUMB-37: two release workflows published the same tag - How found: audit of the release path. `.github/workflows/release.yml` and `.forgejo/workflows/release.yml` both ran on `v*` tag pushes, and both wrote `/srv/breadway-dl/breadcrumbs/<version>` and ran `gh release upload --clobber`. Forgejo is authoritative, since GitHub auto-mirrors from origin. - Fix: the GitHub release workflow is `workflow_dispatch` only (e44b642). The Forgejo release path is unchanged. - Test: `tests/release_workflows.rs`, which asserts the GitHub workflow does not trigger on tag pushes and the Forgejo one still triggers on `v*`. - Note: `CLAUDE.md` is not tracked in this repo (removed in 7a58d4a and listed in `.gitignore` as local-only), so there is no tracked file to rewrite. This card's CLAUDE.md part is therefore not applicable here. Verified locally: `cargo fmt --all -- --check`, `cargo clippy --all-targets -- -D warnings`, `cargo test --release`. Refs CRUMB-25 CRUMB-28 CRUMB-32 CRUMB-35 CRUMB-36 CRUMB-37 CRUMB-44

Breadway added 7 commits 2026-10-20 15:35:17 +08:00

fix(detect): emit one JSON object for detect --apply --json (CRUMB-25) f66fca8a05

fix(status): treat a non-204 HTTP answer with blocked ping as a portal (CRUMB-36) a2e3e2cab7

fix(nm): propagate hidden=false to the saved profile (CRUMB-28) d4e313a8d9

fix(nm): create the ipv6 section when pinning DNS (CRUMB-32) ... 4b5f017269

apply_dns created the ipv4 section for the pin but only touched ipv6 if
it already existed. A profile without an ipv6 section kept DHCP/RA DNS
around the pin. The pin now creates the ipv6 section the same way, with
only DNS keys (no ipv6 method, so NetworkManager keeps its default).
Unpin still only touches sections that exist.

fix(add): decide password keep-or-write from the fresh config (CRUMB-35) ... c3889eaef4

cmd_add computed whether the network existed from the startup config,
before the interactive prompt. The locked Config::update closure reloads
the config, so a network created in the meantime could have its password
wiped by a blank answer. The existence check now runs inside the locked
closure against the fresh config, and the keep-or-write choice is a pure
function (password_to_write) with unit tests for existing vs new and
blank vs non-blank input. The prompt stays outside the lock.

fix(events): select exact profile keys and report whitespace-only payloads (CRUMB-44) ... 319e473578

A set_profile payload was trimmed before the config lookup, so a
configured profile key such as " home " could never be selected. The
payload is now forwarded untrimmed; apply resolves an exact configured
key first and falls back to the trimmed name. A whitespace-only payload
now reports an empty-profile error instead of a missing-field error.

The integration test that asserted the old trim-on-receive behaviour now
asserts the new contract.

ci(release): publish tags only from Forgejo (CRUMB-37) ... e44b642c3d

.github/workflows/release.yml and .forgejo/workflows/release.yml both
ran on v* tag pushes. Both write /srv/breadway-dl/breadcrumbs/<version>
and run 'gh release upload --clobber', so one tag push raced two
publishers. Forgejo is authoritative (GitHub auto-mirrors from origin),
so the GitHub copy is now workflow_dispatch only. The file is kept rather
than deleted so a manual run stays possible; it is the one trigger left.

Not changed: the Forgejo release path, rc-release and dev-release.
CLAUDE.md is not tracked in this repo (removed in 7a58d4a and listed in
.gitignore as local-only), so there is no tracked file here to rewrite.

Adds tests/release_workflows.rs to pin the trigger shape: the GitHub
release workflow must not run on tag pushes, and the Forgejo one must
still run on v*.

Breadway commented 2026-10-20 17:27:59 +08:00

Author

Owner

Copy link

Review of PR #46 at head e44b642c3d (base main 939d9edc). Head re-read from Forgejo before and after review: unchanged.

PR VERDICT: CHANGES REQUESTED (blocking: CRUMB-32; test gap: CRUMB-36). Five cards pass.

Checks on this head: Forgejo "check / check (push)" = success. Independently in a clean worktree: cargo fmt --check rc 0, cargo clippy --all-targets -D warnings rc 0, cargo test rc 0 (113 lib, 54, 56, 2 release_workflows, 0 failures). Existing reviews: none. Branch protection is not readable anonymously, so required-check configuration is unverified.

PER CARD

• CRUMB-25: PASS. detect --apply --json now prints one object {profile, outcome, ok} (src/app.rs cmd_detect, detect_apply_json); the human line and the bare {"profile"} line are suppressed under --json. Superset of the old key, exit code unchanged. Unit test asserts structure and single-line serialization. It tests the builder, not cmd_detect stdout; acceptable.
• CRUMB-28: PASS. profile_needs_update (src/nm.rs ~506) compares net.hidden to the saved flag both ways, missing key = false. Tests cover un-hide, no-op when equal, hidden, password. Non-blocking: connect_verbose now reads GetSettings on every connect to an existing profile (nm.rs ~939-942); previously only when an update was needed, so a GetSettings failure now aborts connects that used to succeed.
• CRUMB-32: CHANGES REQUESTED. See below.
• CRUMB-35: PASS. Existence is recomputed inside the Config::update closure from the fresh config (app.rs, network_exists then password_to_write); the prompt stays outside the lock; explicit --password/--password-stdin semantics preserved. Four tests pin the decision table. The closure wiring itself is verified by reading, not by a test.
• CRUMB-36: CHANGES REQUESTED (test only; the code is correct). src/status.rs:62 (else if answered => Portal) is not covered: I replaced it with else if false && answered and the whole suite stayed green. The only new test covers the helper is_unexpected_http_answer, not connectivity(). tests/flow_watch.rs already has a FakeRunner harness (internet_ok_requires_204_and_falls_back_to_ping); add a case: curl answers 403/404, ping fails, expect Portal; curl 000 and 500 with ping failing, expect NoNet; 403 with ping succeeding, expect Online. Nit: the Connectivity doc comment still lists only 200/redirect/511.
• CRUMB-37: PASS. .github/workflows/release.yml is workflow_dispatch only; .forgejo release.yml (v*, non-rc) and rc-release.yml (v*, rc only) are mutually exclusive. tests/release_workflows.rs asserts both sides. CLAUDE.md: untracked at base (git cat-file fails at 939d9edc; removed in 7a58d4a; listed in .gitignore), so that half is correctly not applicable. Non-blocking hazard: the GitHub workflow still derives VERSION from GITHUB_REF_NAME, so a manual dispatch from a branch writes /srv/breadway-dl/breadcrumbs/ and repoints the "latest" symlink at it. Consider deleting that workflow or guarding the job with if: startsWith(github.ref, 'refs/tags/v').
• CRUMB-44: PASS. Payload forwarded untrimmed; exact configured key wins, trimmed is the fallback (resolve_profile_key); whitespace-only reports "empty profile" instead of "missing string". Unit tests plus the renamed integration test assert the new contract. Nit: the bread.crumbs.set_profile.done event (bread_events.rs:147) echoes the raw payload, not the resolved key.

CRUMB-32 (blocking)
Change: apply_dns (src/nm.rs:857-858) now creates an ipv6 section containing only ignore-auto-dns and dns, with no method key. Tests assert the absence of method on purpose (nm.rs:1226 and :1237).
Evidence that this is invalid to NetworkManager:

1. libnm source (NetworkManager main, nm-setting-ip-config.c verify): if (!priv->method) fail with "ipv6.method: property is missing". The method property has no default (g_param_spec_string default NULL), and nm-setting-ip6-config.c init sets none. The documented "auto" default applies only when the whole ipv6 setting is absent (nm-connection.c _normalize_ip_config adds ipv6 method=auto, may-fail=TRUE). A present-but-methodless section is not defaulted.
2. Update2 parses with STRICT|NORMALIZE (nm-settings-connection.c settings_connection_update), and the keyfile reader also normalizes, so the invalid profile is rejected, not repaired.
3. Reproduced in-process with the host libnm 1.46.0 (no daemon, no D-Bus, no NMClient): ipv6 {ignore-auto-dns, dns} without method => "ipv6.method: property is missing"; the same dict with method=auto verifies and normalizes. Same failure for the pre-existing ipv4 shape without method.
4. A profile with no [ipv6] in keyfile, loaded and normalized, comes back with ipv6.method=auto and an ipv6 section in GetSettings-shaped output. So on a live system GetSettings should essentially always include ipv6 with a method, which makes the leak scenario in the card largely theoretical.
   Consequence: if the hypothetical profile ever does occur, enforce_dns's Update2 fails, the error is only logged ("dns: could not apply DNS setting"), and the v4 pin that worked before this change is lost too. That is a regression in the one case the change targets.
   The fake does not catch it: tests/common/fake_nm.rs validate_settings (line 206) checks dns wire types but not that ip sections carry a method, so the tests mirror the code.
   Requested: when creating a missing ipv6 section, also set method=auto (as the create path at nm.rs:697-699 does); flip the two assertions to expect method == "auto"; teach validate_settings to reject ipv4/ipv6 sections without method, so a regression fails in the fake.
   Does pinning stop v6 auto DNS? With method=auto, per nm-settings docs ipv6.ignore-auto-dns=TRUE ignores RA/DHCPv6 name servers and search domains, and only dns/dns-search are used; so yes for this profile's own DNS. Not covered: dns-priority is never set, so other active connections (VPN, second NIC) can still contribute DNS; the card does not claim that. Separate pre-existing issue, not for this PR: with ipv6.method ignore, disabled or link-local, libnm rejects a non-empty ipv6.dns, so a pin with a v6 server on such a profile fails the same way (reproduced in-process).
   Confidence: high that methodless ipv6 is rejected (source plus libnm 1.46.0 run); medium-high that the no-ipv6-section case rarely occurs live. Only provable on a live system: the running daemon's behavior (same libnm code, but I did not run NetworkManager), real RA/RDNSS and DHCPv6 DNS being dropped after Reapply, resulting resolv.conf or systemd-resolved state, and interaction with Tailscale DNS. None of that was touched.

Scope: seven files, all attributable to the seven cards; no scope creep. Added lines and commit messages contain no em or en dashes and no attribution.
Not merged. Cards not moved.

Review of PR #46 at head e44b642c3d8129f5d06eb9df1c57a046977212b9 (base main 939d9edc). Head re-read from Forgejo before and after review: unchanged. PR VERDICT: CHANGES REQUESTED (blocking: CRUMB-32; test gap: CRUMB-36). Five cards pass. Checks on this head: Forgejo "check / check (push)" = success. Independently in a clean worktree: cargo fmt --check rc 0, cargo clippy --all-targets -D warnings rc 0, cargo test rc 0 (113 lib, 54, 56, 2 release_workflows, 0 failures). Existing reviews: none. Branch protection is not readable anonymously, so required-check configuration is unverified. PER CARD - CRUMB-25: PASS. detect --apply --json now prints one object {profile, outcome, ok} (src/app.rs cmd_detect, detect_apply_json); the human line and the bare {"profile"} line are suppressed under --json. Superset of the old key, exit code unchanged. Unit test asserts structure and single-line serialization. It tests the builder, not cmd_detect stdout; acceptable. - CRUMB-28: PASS. profile_needs_update (src/nm.rs ~506) compares net.hidden to the saved flag both ways, missing key = false. Tests cover un-hide, no-op when equal, hidden, password. Non-blocking: connect_verbose now reads GetSettings on every connect to an existing profile (nm.rs ~939-942); previously only when an update was needed, so a GetSettings failure now aborts connects that used to succeed. - CRUMB-32: CHANGES REQUESTED. See below. - CRUMB-35: PASS. Existence is recomputed inside the Config::update closure from the fresh config (app.rs, network_exists then password_to_write); the prompt stays outside the lock; explicit --password/--password-stdin semantics preserved. Four tests pin the decision table. The closure wiring itself is verified by reading, not by a test. - CRUMB-36: CHANGES REQUESTED (test only; the code is correct). src/status.rs:62 (else if answered => Portal) is not covered: I replaced it with else if false && answered and the whole suite stayed green. The only new test covers the helper is_unexpected_http_answer, not connectivity(). tests/flow_watch.rs already has a FakeRunner harness (internet_ok_requires_204_and_falls_back_to_ping); add a case: curl answers 403/404, ping fails, expect Portal; curl 000 and 500 with ping failing, expect NoNet; 403 with ping succeeding, expect Online. Nit: the Connectivity doc comment still lists only 200/redirect/511. - CRUMB-37: PASS. .github/workflows/release.yml is workflow_dispatch only; .forgejo release.yml (v*, non-rc) and rc-release.yml (v*, rc only) are mutually exclusive. tests/release_workflows.rs asserts both sides. CLAUDE.md: untracked at base (git cat-file fails at 939d9edc; removed in 7a58d4a; listed in .gitignore), so that half is correctly not applicable. Non-blocking hazard: the GitHub workflow still derives VERSION from GITHUB_REF_NAME, so a manual dispatch from a branch writes /srv/breadway-dl/breadcrumbs/<branch> and repoints the "latest" symlink at it. Consider deleting that workflow or guarding the job with if: startsWith(github.ref, 'refs/tags/v'). - CRUMB-44: PASS. Payload forwarded untrimmed; exact configured key wins, trimmed is the fallback (resolve_profile_key); whitespace-only reports "empty profile" instead of "missing string". Unit tests plus the renamed integration test assert the new contract. Nit: the bread.crumbs.set_profile.done event (bread_events.rs:147) echoes the raw payload, not the resolved key. CRUMB-32 (blocking) Change: apply_dns (src/nm.rs:857-858) now creates an ipv6 section containing only ignore-auto-dns and dns, with no method key. Tests assert the absence of method on purpose (nm.rs:1226 and :1237). Evidence that this is invalid to NetworkManager: 1. libnm source (NetworkManager main, nm-setting-ip-config.c verify): if (!priv->method) fail with "ipv6.method: property is missing". The method property has no default (g_param_spec_string default NULL), and nm-setting-ip6-config.c init sets none. The documented "auto" default applies only when the whole ipv6 setting is absent (nm-connection.c _normalize_ip_config adds ipv6 method=auto, may-fail=TRUE). A present-but-methodless section is not defaulted. 2. Update2 parses with STRICT|NORMALIZE (nm-settings-connection.c settings_connection_update), and the keyfile reader also normalizes, so the invalid profile is rejected, not repaired. 3. Reproduced in-process with the host libnm 1.46.0 (no daemon, no D-Bus, no NMClient): ipv6 {ignore-auto-dns, dns} without method => "ipv6.method: property is missing"; the same dict with method=auto verifies and normalizes. Same failure for the pre-existing ipv4 shape without method. 4. A profile with no [ipv6] in keyfile, loaded and normalized, comes back with ipv6.method=auto and an ipv6 section in GetSettings-shaped output. So on a live system GetSettings should essentially always include ipv6 with a method, which makes the leak scenario in the card largely theoretical. Consequence: if the hypothetical profile ever does occur, enforce_dns's Update2 fails, the error is only logged ("dns: could not apply DNS setting"), and the v4 pin that worked before this change is lost too. That is a regression in the one case the change targets. The fake does not catch it: tests/common/fake_nm.rs validate_settings (line 206) checks dns wire types but not that ip sections carry a method, so the tests mirror the code. Requested: when creating a missing ipv6 section, also set method=auto (as the create path at nm.rs:697-699 does); flip the two assertions to expect method == "auto"; teach validate_settings to reject ipv4/ipv6 sections without method, so a regression fails in the fake. Does pinning stop v6 auto DNS? With method=auto, per nm-settings docs ipv6.ignore-auto-dns=TRUE ignores RA/DHCPv6 name servers and search domains, and only dns/dns-search are used; so yes for this profile's own DNS. Not covered: dns-priority is never set, so other active connections (VPN, second NIC) can still contribute DNS; the card does not claim that. Separate pre-existing issue, not for this PR: with ipv6.method ignore, disabled or link-local, libnm rejects a non-empty ipv6.dns, so a pin with a v6 server on such a profile fails the same way (reproduced in-process). Confidence: high that methodless ipv6 is rejected (source plus libnm 1.46.0 run); medium-high that the no-ipv6-section case rarely occurs live. Only provable on a live system: the running daemon's behavior (same libnm code, but I did not run NetworkManager), real RA/RDNSS and DHCPv6 DNS being dropped after Reapply, resulting resolv.conf or systemd-resolved state, and interaction with Tailscale DNS. None of that was touched. Scope: seven files, all attributable to the seven cards; no scope creep. Added lines and commit messages contain no em or en dashes and no attribution. Not merged. Cards not moved.

Breadway added 2 commits 2026-10-20 17:44:10 +08:00

fix(nm): give a created ipv4/ipv6 section method auto (CRUMB-32) ... d4db5b874a

NetworkManager rejects an ipv4 or ipv6 section that has no method
("ipv6.method: property is missing"), so the DNS pin's created sections
made verify and Update2 fail and the pin never took effect. Created
sections now carry method = auto, the default NM applies to a profile
with no section at all. Existing sections keep their method.

The fake NM now rejects a present ipv4/ipv6 section without a method,
so a client that builds an invalid shape fails in tests the way it
would against real NetworkManager.

test(status): cover the non-204 HTTP answer with a blocked ping (CRUMB-36) ... 2dff10b59c

Pin the connectivity verdicts through the FakeRunner harness: a 403 or
404 with ping blocked is a portal, 000 or 500 with ping blocked is no
network, and a 403 with ping working is online. Breaking the portal
branch in status.rs fails the 403/404 test.

Breadway commented 2026-10-20 17:44:13 +08:00

Author

Owner

Copy link

Rework for the independent review (comment 5672), on the same branch.

• d4db5b8 fix(nm): give a created ipv4/ipv6 section method auto (CRUMB-32). The DNS pin now creates a missing ipv6 section with method = auto. The same missing-method path existed for ipv4, so that is fixed too. Existing sections keep their method. The assertions at the two previously flagged spots now expect auto. The fake NM's validate_settings rejects a present ipv4/ipv6 section with no method, with unit tests for reject and accept.
• 2dff10b test(status): cover the non-204 HTTP answer with a blocked ping (CRUMB-36). Tests of connectivity() through FakeRunner: 403/404 with ping blocked gives Portal, 000/500 with ping blocked gives NoNet, 403 with ping working gives Online. Breaking the portal branch to else if false && answered fails the 403/404 test; restored afterwards.

Checks on 2dff10b: cargo fmt --check, cargo clippy --all-targets -- -D warnings, cargo test --release all pass.

Rework for the independent review (comment 5672), on the same branch. - d4db5b8 fix(nm): give a created ipv4/ipv6 section method auto (CRUMB-32). The DNS pin now creates a missing ipv6 section with `method = auto`. The same missing-method path existed for ipv4, so that is fixed too. Existing sections keep their method. The assertions at the two previously flagged spots now expect `auto`. The fake NM's `validate_settings` rejects a present ipv4/ipv6 section with no `method`, with unit tests for reject and accept. - 2dff10b test(status): cover the non-204 HTTP answer with a blocked ping (CRUMB-36). Tests of `connectivity()` through FakeRunner: 403/404 with ping blocked gives Portal, 000/500 with ping blocked gives NoNet, 403 with ping working gives Online. Breaking the portal branch to `else if false && answered` fails the 403/404 test; restored afterwards. Checks on 2dff10b: cargo fmt --check, cargo clippy --all-targets -- -D warnings, cargo test --release all pass.

All checks were successful

Hide all checks

check / check (push) Successful in 5m37s

Details

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin fix/crumb-audit-batch:fix/crumb-audit-batch

git switch fix/crumb-audit-batch

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main

git merge --no-ff fix/crumb-audit-batch

git switch fix/crumb-audit-batch

git rebase main

git switch main

git merge --ff-only fix/crumb-audit-batch

git switch fix/crumb-audit-batch

git rebase main

git switch main

git merge --no-ff fix/crumb-audit-batch

git switch main

git merge --squash fix/crumb-audit-batch

git switch main

git merge --ff-only fix/crumb-audit-batch

git switch main

git merge fix/crumb-audit-batch

git push origin main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

bug

  

ci

  

enhancement

No labels

bug

ci

enhancement

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Breadway/breadcrumbs!46

No description provided.