Prefer Merlin dnsmasq hooks and fix Guest Network Pro integration

[maghuro]

Important

Local staging/review draft in the fork only. This PR is stacked on the final review-clean #1 branch. Per upstream contribution guidelines, an upstream issue must be filed before any PR is submitted to Control-D-Inc/ctrld.

Summary

Move Asuswrt-Merlin's ctrld forwarding integration to the supported hook model, add native Merlin 3006 Guest Network Pro / SDN support, and make migration/fallback recovery transactional.

Main changes

• apply Merlin NVRAM changes before regenerating dnsmasq, with Merlin-specific transactional rollback/retry handling;
• install ctrld-owned logic in both dnsmasq.postconf and Merlin 3006's dnsmasq-sdn.postconf;
• restart dnsmasq and verify the generated runtime configuration has the complete expected ctrld forwarding shape;
• use dnsmasq-sdn.postconf for Guest Network Pro / SDN instances and never create unsupported full /jffs/configs/dnsmasq-N.conf replacements;
• retain the old full-config mechanism only as a compatibility fallback for the main dnsmasq when the supported hook demonstrably failed;
• bind fallback ownership to a private hard-link anchor for the exact inode ctrld published, rather than trusting pathname/content equality alone;
• journal fallback publication, quarantine, deletion and restoration through durable phases so interrupted operations are retryable after power loss;
• use no-clobber publication/restoration and preserve concurrent user/addon files;
• revalidate both inode identity and content immediately before destructive fallback finalization;
• migrate legacy ctrld-created main/numeric dnsmasq snapshots through a durable path+hash cleanup journal without re-enumerating newly-created files on retry;
• fsync files and parent directories where ordering/durability matters, including retrying uncertain deletions;
• validate that /tmp/ctrld.pid still belongs to a ctrld process before the Merlin postconf block rewrites DNS.

Why

Current ctrld discovers /tmp/etc/dnsmasq-*.conf and writes complete copies under /jffs/configs/dnsmasq-N.conf.

On Merlin 3006, the supported SDN extension points are:

• dnsmasq-N.conf.add
• dnsmasq-sdn.postconf <config-path> <sdn-index>

The Merlin 3006 SDN code appends the indexed .add file and runs dnsmasq-sdn.postconf; it does not consume a full /jffs/configs/dnsmasq-N.conf replacement. This matches the failure reported in Control-D-Inc#229.

For the main LAN, a complete /jffs/configs/dnsmasq.conf also freezes firmware-generated state. On a GT-AX6000 / Merlin 3006 reproduction, temporarily removing that full override and restarting dnsmasq generated a valid native config while ctrld's postconf logic still applied correctly.

Verification model

A runtime dnsmasq config is accepted only when it has the expected forwarding shape:

• exactly one server= directive and it matches the active ctrld listener;
• no-resolv;
• no servers-file= or resolv-file=;
• no DNSSEC/trust-anchor fallback path;
• add-mac;
• add-subnet=32,128;
• cache-size=0.

Main-hook failure may invoke the compatibility fallback only when ctrld can prove ownership safely. SDN-hook failure is reported instead of writing an unsupported full SDN snapshot.

Scope note

The pre-existing transition to a direct wildcard DNS listener on port 53 has a separate port-ownership/lifecycle problem. This PR deliberately does not claim to solve that broader transition.

Related

• AsusRT firmware (3006 base) Control-D-Inc/ctrld#229
• stacked on Preserve Asuswrt-Merlin dnsmasq postconf hooks #1

[maghuro]

@codex review

Please review this stacked Merlin dnsmasq/Guest Network Pro change with particular attention to:

• safety of legacy snapshot migration vs user-owned /jffs/configs/dnsmasq*.conf files;
• setup rollback and cleanup retryability;
• ordering of nvram.SetKV vs dnsmasq regeneration;
• whether runtime verification of server=# is sufficient;
• fallback ownership state and any path that could delete an unowned custom config;
• Merlin 3006 SDN behavior via dnsmasq-sdn.postconf;
• transitions between dnsmasq-forwarder mode and a direct port-53 listener.

Please also flag any behavior that could leave DNS broken after a partial setup/cleanup failure.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a7d34433df

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[maghuro]

@codex review

The stacked base has advanced and hook cleanup now never deletes shared hook paths. Please review current head 474bd34, especially legacy snapshot migration, cleanup retryability, fallback ownership, and Guest Network Pro/SDN behavior.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 474bd34cec

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[maghuro]

@codex review

Current head includes a substantial lifecycle/failure-recovery hardening pass on top of the latest #1 hook logic:

• Setup now propagates NVRAM setup-state read failures instead of treating them as unset;
• SetKV explicitly rolls back partial NVRAM mutations; Restore keeps backup keys until restored state has committed;
• PreRun retries transient cleanup failures instead of abandoning the one-shot router startup immediately;
• direct-listener cleanup remains intentionally skipped so dnsmasq does not reclaim port 53 before ctrld binds;
• runtime verification now requires the full ctrld forwarding shape (single expected server, no alternate resolv/servers-file/DNSSEC path, expected metadata/cache directives);
• legacy cleanup uses durable cleanup/finalize phases so marker-removal failures cannot cause later deletion of newly created custom snapshots;
• main-LAN fallback ownership is hash-bound and published before the snapshot, with user modifications protected from overwrite/removal;
• Guest Network Pro / SDN remains hook-only via dnsmasq-sdn.postconf;
• PR Preserve Asuswrt-Merlin dnsmasq postconf hooks #1's latest reversible/atomic shared-hook machinery is synced into this branch.

Please review current head 7b40f1b, with particular focus on compile/test consistency, NVRAM rollback semantics, cleanup phases, fallback ownership/crash windows, direct-listener transitions, and SDN behavior.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7b40f1bf6f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[maghuro]

@codex review

Latest head ed6d61a incorporates the previous findings and resyncs the current #1 transaction layer:

• positive dnsmasq verification fixtures include the complete required forwarding shape;
• SetKV keeps ctrld_setup armed whenever rollback cannot restore a value, and Restore re-arms it if the restore commit fails;
• legacy migration now journals the exact main + dnsmasq-N snapshot paths and SHA-256 hashes before deleting anything; retries use only that durable journal and never re-enumerate newly created files;
• journal parsing restricts entries to the expected /jffs/configs/dnsmasq.conf and numeric dnsmasq-N.conf namespace, with round-trip and unsafe-path tests;
• current Preserve Asuswrt-Merlin dnsmasq postconf hooks #1 precommit path/content/symlink revalidation and deterministic rollback tests are synced.

Please review current head from scratch, especially compile/test consistency, crash windows in the legacy journal/fallback ownership, NVRAM retryability, stacked-branch consistency, and SDN/Guest Network Pro behavior. The direct-listener-on-port-53 lifecycle is pre-existing behavior and is being audited separately rather than silently mixed into this forwarder/SDN patch.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ed6d61acc2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[maghuro]

@codex review

Addressed the latest review findings on current head 186d8c8:

• removed the unused NVRAM commit output binding that broke compilation;
• atomic Merlin writes now fsync the parent directory after rename (with Windows CI compatibility), so marker-first fallback ownership is actually durable across power loss;
• legacy cleanup now journals exact snapshot paths + hashes before any deletion, including stale numeric dnsmasq-N snapshots;
• NVRAM rollback/restore paths keep or re-arm ctrld_setup when restoration cannot be made durable.

Please review the current head from scratch for compile/test issues, crash durability, legacy cleanup safety, fallback ownership, NVRAM recovery and SDN behavior. Note that PR #2 is still a draft stacked branch and will be rebased/synchronized onto the final narrow #1 head once #1 review is clean.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 186d8c8c5a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[maghuro]

@codex review

Current head 7630422 now also includes:

• error-aware managed-state existence checks, so JFFS I/O errors cannot be misclassified as legacy installs;
• durable fsync ordering for legacy snapshot deletions before advancing the journal;
• fallback ownership retention when rename may have succeeded but directory fsync failed;
• no persistent ctrld_setup=1 re-arm after restored NVRAM values have been applied;
• the latest shared-hook subshell isolation and newline regression from Preserve Asuswrt-Merlin dnsmasq postconf hooks #1;
• Windows-safe POSIX mode tests and cleanup revalidation coverage.

Please review this head from scratch. The PR is still intentionally stacked on #1 and will be cleanly rebuilt onto the final #1 head after #1 is review-clean.

[maghuro]

@codex review

The stacked branch has now been rebuilt cleanly as a single commit directly on the review-clean #1 head (4de5592f), eliminating the historical merge divergence.

Please review current head f402d736 from scratch. Focus especially on:

• compile/test consistency after the clean rebuild;
• Merlin 3006 SDN hook behavior;
• NVRAM setup/restore rollback semantics;
• exact-path/hash legacy snapshot journaling;
• fallback ownership and power-loss ordering;
• error handling around JFFS markers and shared hooks.

The direct wildcard-port-53 listener transition is explicitly out of scope and documented in the PR body.

[maghuro]

@codex review

One more shared-hook safety fix landed after the clean rebuild: cleanup of both main and SDN shared hooks now uses the same prepare + path/type/symlink/content revalidation model as setup, instead of blindly rewriting a hook that may have changed concurrently.

Please review current head d4058d7 rather than the earlier rebuilt head.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d4058d7511

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[maghuro]

@codex review

Addressed the remaining P1 from the previous pass on current head 3f97e77:

• if Restore's persistent nvram commit fails after ctrld_setup was unset in the volatile view, ctrld now re-arms ctrld_setup=1 only in volatile NVRAM;
• it deliberately does not commit that marker, so same-boot Cleanup retries remain possible without risking resurrection of a completed transaction after reboot;
• if even the volatile re-arm fails, that error is joined with the original commit failure.

Please review the current head from scratch, including the newer shared-hook rollback revalidation commit and this NVRAM recovery path.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3f97e7769b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[maghuro]

@codex review

Addressed the cross-router P1 on current head f058e01:

• shared nvram.Restore keeps its original non-rearming behavior for DD-WRT/FreshTomato and other callers;
• new RestoreWithVolatileRetryMarker is explicitly Merlin-only and delegates to the same restore core with same-boot volatile re-arm enabled;
• Merlin Cleanup is the only caller switched to the retry-aware variant;
• no persistent marker is written by the retry-aware recovery path.

Please review the current head from scratch, including compile consistency and router-backend isolation.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f058e01f46

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[maghuro]

@codex review

One additional fail-closed guard on current head 9dc944c:

• initial legacy-journal creation now refuses any unjournaled .dnsmasq*.ctrld-legacy-quarantine path instead of adopting private-looking data as ctrld-owned state.

Please include this in the full review requested above.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9dc944c79e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[maghuro]

Addressed the last Codex P2 manually on current head 380e948 while code-review credits are exhausted:

• legacy snapshot identity is now captured before journal publication via a private hard-link anchor under /jffs/controld;
• the anchor is verified against the inode opened during capture with os.SameFile, and content is re-read to reject in-place modification during capture;
• orphan anchors from a crash before journal publication are safe to remove because they are ctrld-private extra hard links only;
• pending cleanup never infers ownership from path+hash: public/quarantine files must still be the same inode as the captured anchor and retain the journaled hash;
• byte-identical replacement after journal creation is therefore preserved;
• an anchorless quarantine is restored rather than forgotten;
• added regression tests for byte-identical inode replacement and captured hard-link ownership.

Codex review is currently rate-limited, so this head is undergoing manual adversarial review until review credits return.

[maghuro]

@codex review

Credits may be available again. Please perform a fresh full review of current head 5171b79ede and report any remaining correctness, compile/test, crash-recovery, race, portability, or lifecycle issues.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5171b79ede

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[maghuro]

@codex review

Addressed both findings from the last review on current head 59f51c0:

• Merlin now ensures its private state directory /jffs/controld exists before PreRun reconciliation, Setup, or standalone Cleanup, so supported installs with the ctrld binary elsewhere under /jffs do not fail on state-file creation;
• dnsmasq verification now fails closed on conf-file=, conf-dir=, and conf-script= directives instead of declaring the top-level config safe while unresolved included configuration may add bypass upstreams;
• added verifier regressions for all three include forms.

Please review the full current head again, including compile/test consistency and any remaining lifecycle/crash/race issues.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 59f51c00a2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[maghuro]

@codex review

Current head is now a83b4ec.

The CI-only failure from the previous run was a test fixture issue: os.WriteFile obeyed the runner umask, so the mode-preservation test was asserting against a mode the fixture never actually had. The fixture now chmods explicitly before exercising atomicWriteFile.

Please review/test the current head, including the earlier state-directory and dnsmasq include-directive fixes.

[maghuro]

@codex review

Addressed the state-directory crash-durability finding on current head fcd016c:

• ensureMerlinStateDir now distinguishes existing vs newly-created state directories;
• when it actually creates /jffs/controld, it fsyncs /jffs before any child state file is relied upon;
• it also rejects a non-directory state path explicitly.

Please review the full current head again.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fcd016c5fa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[maghuro]

@codex review

Addressed the retry durability window on exact current head cf269e2:

• ensureMerlinStateDir now fsyncs /jffs before returning even when /jffs/controld already exists;
• this means a retry after mkdir-success + parent-fsync-failure must reconfirm the directory entry's durability before any child state is trusted;
• non-directory and stat-error behavior is unchanged.

Please perform a fresh full review of this exact head.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: cf269e253d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[maghuro]

Final local readiness note:

• current head: cf269e253d;
• this exact head received Codex: "Didn't find any major issues. More of your lovely PRs please.";
• current-head CI passes on Ubuntu and Windows;
• macOS fails in WillAbides/setup-go-faster@v1.8.0 before repository tests start.

No material Codex finding remains unaddressed.

[maghuro]

Superseded by the maintained v1.x port in #7, now merged into v1.0 on top of #6. This main-based draft is retained only as review history/source-of-truth for the port.