Dry-run preflight doesn't check exit_on_bootup() for --reconcile #20

New issue

Closed

opened 2026-10-22 17:36:12 -07:00 by eric · 1 comment

eric commented

2026-10-22 17:36:12 -07:00

Owner

Summary

--reconcile skips during boot through exit_on_bootup(), but only if the function exists:

if ($reconcile && !$dryrun && function_exists('exit_on_bootup')) {
    exit_on_bootup();
}

(around line 599). If OPNsense ever renames or removes it, reconcile silently loses its boot guard and races rc.bootup bringing interfaces up. The dry-run preflight, which is supposed to catch exactly this ("every OPNsense PHP API it calls exists on this build"), doesn't check it.

It exists today: src/etc/inc/util.inc in opnsense/core, function exit_on_bootup($callback = null, $arguments = []), which calls exit(0) when product::getInstance()->booting().

Fix

In preflight() (around lines 529–534), add exit_on_bootup to $apis when --reconcile is in effect, the same way system_routing_configure is added when $remove_backup_route is set. preflight() doesn't currently receive $reconcile, so add a parameter (or use the global, matching how wlog/werr read $dryrun). Update the call site (around line 641). The existing "MISSING — a LIVE run would fatal" wording is wrong for this one: a missing exit_on_bootup doesn't fatal, it silently skips the boot guard. Give it its own message.

Optionally, also log a werr on a live reconcile when the function is missing, so the gap shows up in the log and not only in dry-run.

Update the README Dry-run preflight bullet list if it names the APIs, and add a CHANGELOG [Unreleased] entry.

Acceptance

--dryrun --reconcile prints API exit_on_bootup() present.; --dryrun 1@x MASTER doesn't list it.

## Summary `--reconcile` skips during boot through `exit_on_bootup()`, but only if the function exists: ```php if ($reconcile && !$dryrun && function_exists('exit_on_bootup')) { exit_on_bootup(); } ``` (around line 599). If OPNsense ever renames or removes it, reconcile silently loses its boot guard and races `rc.bootup` bringing interfaces up. The dry-run preflight, which is supposed to catch exactly this ("every OPNsense PHP API it calls exists on this build"), doesn't check it. It exists today: `src/etc/inc/util.inc` in opnsense/core, `function exit_on_bootup($callback = null, $arguments = [])`, which calls `exit(0)` when `product::getInstance()->booting()`. ## Fix In `preflight()` (around lines 529–534), add `exit_on_bootup` to `$apis` when `--reconcile` is in effect, the same way `system_routing_configure` is added when `$remove_backup_route` is set. `preflight()` doesn't currently receive `$reconcile`, so add a parameter (or use the global, matching how `wlog`/`werr` read `$dryrun`). Update the call site (around line 641). The existing "MISSING — a LIVE run would fatal" wording is wrong for this one: a missing `exit_on_bootup` doesn't fatal, it silently skips the boot guard. Give it its own message. Optionally, also log a `werr` on a live reconcile when the function is missing, so the gap shows up in the log and not only in dry-run. Update the README Dry-run preflight bullet list if it names the APIs, and add a CHANGELOG `[Unreleased]` entry. ## Acceptance - `--dryrun --reconcile` prints `API exit_on_bootup() present.`; `--dryrun 1@x MASTER` doesn't list it.

eric added the

enhancement

label

2026-10-22 17:36:12 -07:00

eric referenced this issue from a commit

2026-10-23 15:54:00 -07:00

fix: check exit_on_bootup() in the dry-run preflight for --reconcile

eric closed this issue

2026-10-23 15:54:00 -07:00

eric referenced this issue from a commit

2026-10-23 16:06:45 -07:00

fix: follow-up audit of #15, #18 and #20

eric commented

2026-10-23 16:07:31 -07:00

Author

Owner

Follow-up audit: a missing exit_on_bootup() would flood the log

Fixed in 222180a. Low impact: current core has exit_on_bootup() (util.inc), so this only affects old builds.

Finding. On a build without exit_on_bootup(), a live reconcile logged the gap with werr(). That sets $had_error, and a quiet reconcile flushes its buffered log whenever $had_error is set, so every once-a-minute reconcile would have written its whole log to the system log, defeating the quiet-reconcile design.

Fix.

Without exit_on_bootup(), a live reconcile now makes the same check itself (product::booting()) and exits during boot, silently.
Only if product is missing too does it note no boot guard on this build ..., and that note goes through wlog(), so it only appears in a run that is logged anyway (one that changed something or hit a real error).
The dry-run preflight says exit_on_bootup() missing; a LIVE reconcile uses product::booting() as its boot guard., and is an error only when neither is available.

Harness: the old logs a missing exit_on_bootup() cases were replaced by six covering the fallback during boot and after boot, the quiet no-guard reconcile, the no-guard note on a run that fixes something, and the two dry-run messages. No change to the live test plan: OPNsense builds that the script supports all have exit_on_bootup().

## Follow-up audit: a missing exit_on_bootup() would flood the log Fixed in 222180a. Low impact: current core has `exit_on_bootup()` (`util.inc`), so this only affects old builds. **Finding.** On a build without `exit_on_bootup()`, a live reconcile logged the gap with `werr()`. That sets `$had_error`, and a quiet reconcile flushes its buffered log whenever `$had_error` is set, so every once-a-minute reconcile would have written its whole log to the system log, defeating the quiet-reconcile design. **Fix.** - Without `exit_on_bootup()`, a live reconcile now makes the same check itself (`product::booting()`) and exits during boot, silently. - Only if `product` is missing too does it note `no boot guard on this build ...`, and that note goes through `wlog()`, so it only appears in a run that is logged anyway (one that changed something or hit a real error). - The dry-run preflight says `exit_on_bootup() missing; a LIVE reconcile uses product::booting() as its boot guard.`, and is an error only when neither is available. Harness: the old `logs a missing exit_on_bootup()` cases were replaced by six covering the fallback during boot and after boot, the quiet no-guard reconcile, the no-guard note on a run that fixes something, and the two dry-run messages. No change to the live test plan: OPNsense builds that the script supports all have `exit_on_bootup()`.

eric referenced this issue

2026-10-23 16:07:31 -07:00

Release test plan: live checks before tagging the next release #21