Skip to content

feat: Composable prestart hook (Plan A: capability + docs)#212

Merged
mairas merged 4 commits into
mainfrom
feat/composable-prestart-hook
Jun 15, 2026
Merged

feat: Composable prestart hook (Plan A: capability + docs)#212
mairas merged 4 commits into
mainfrom
feat/composable-prestart-hook

Conversation

@mairas

@mairas mairas commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Motivation

generate_prestart_file copied a custom prestart.sh verbatim and skipped
generation, so any app needing one custom step had to re-implement all framework
boilerplate (runtime dir, env sourcing, HOSTNAME/HOMARR_URL). That
duplicated-boilerplate class is what shipped the HALOS_DOMAIN bug (#177). This
is Plan A (#207): the capability + docs, shipped first and independently of
declarative OIDC (Plan B, #211) and the legacy cleanup (Plan C, #210).

Approach

  • The generated prestart.sh is now always the framework scaffolding and
    ends by sourcing an optional app-prestart.sh hook. An app that ships a
    prestart.sh has it copied into the package as app-prestart.sh, beside
    docker-compose.yml (so hooks resolving their own dir via BASH_SOURCE
    still find the compose file).
  • runtime.env is framework-owned: created mode 600 (install -m 600 /dev/null)
    before any write, so the mode predates content and survives every append. The
    hook contract is append-only to $RUNTIME_ENV.
  • HOSTNAME is dropped from the container env (HOMARR_URL is built from
    ${HALOS_DOMAIN}, not hostname -s, so it was unused).
  • set -e propagates hook failures to unit start.
  • Documented the hook contract ($RUNTIME_ENV append-only, $HALOS_DOMAIN,
    sourced env, hook dir, set -e semantics, static seed → default-data/) in
    AGENTS.md / EXAMPLES.md.

Scope

Capability + docs only. No OIDC work (Plan B), no legacy traefik/oidc_snippet
removal (Plan C), and no marine app migration (that lands as one coordinated PR
after Plan B also ships — the marine repo builds all apps under a single
CONTAINER_TOOLS_REF).

Verification

tests/test_prestart.py / test_builder.py: plain and custom-logic apps both
get framework scaffolding + the hook-source line; runtime.env is 600;
app-prestart.sh is a sibling of docker-compose.yml; a compliant append-only
hook coexists with framework HOSTNAME/HOMARR_URL lines.

Implements Plan A Units 1 & 4 of #207.

mairas added 4 commits June 15, 2026 21:06
…hook

Always generate the framework prestart.sh; a custom prestart.sh becomes a
sourced app-prestart.sh hook installed beside docker-compose.yml, instead of
the all-or-nothing replacement. Create runtime.env mode 600 before any write
(survives appends), drop the unused HOSTNAME write from the container env, and
gate the hook install on has_app_prestart.
…doc tmpfs secret note

Address code-review findings: assert the hook is sourced after the last
runtime.env append (not the declaration); guard the no-web_ui branch against a
truncating write; cover HOMARR_URL+hook coexistence; pin the rules.j2 install
dir to the prestart source dir; document that runtime.env is tmpfs-recreated so
hook secrets must persist to the data dir.
@mairas

mairas commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

Code review — composable prestart hook

Automated multi-persona review (correctness, testing, maintainability, project-standards, security, reliability, adversarial lenses) over the diff.

Verdict: Ready to merge. The capability is correct and well-tested: the 600-before-write is atomic with no window and the sourced hook path isn't injectable (package_name is regex-validated); the set -e guard no-ops on an absent hook; BASH_SOURCE resolution holds; and the HOSTNAME removal breaks no in-tree consumer. The headline P1/P2 are integration concerns inherent to the composable design, explicitly gated by Plan A's capability-first rollout (the marine repo stays pinned until Units 2–3 migrate all four hooks to append-only). The actionable test/doc findings are fixed in 663c818.

P1 — High (tracked, not a defect in this diff)

# File Issue Route
1 prestart.py Unmigrated marine hooks (grafana/signalk cat > runtime.env) truncate the framework-written HOMARR_URL when sourced. The append-only contract is prose, not enforced. Tracked: Plan A Units 2–3 (rollout pin + per-app >> conversion). Hardening recommended (see below).

P2 — Moderate

# File Issue Route
2 prestart.py (hook) set -e is inherited by the sourced hook; previously best-effort steps (e.g. signalk bcrypt) now hard-fail unit start. Tracked: per-hook set -e audit during marine migration; documented in AGENTS.md.
3 prestart.py runtime.env is tmpfs, recreated each start; a hook writing a secret only there regenerates it every restart. Fixed — AGENTS.md now tells hook authors to persist secrets to the data dir with a generate-once guard.
4 rules.j2 / prestart.py Hook install dir and prestart source dir are independent literals; silent runtime-only drift if paths.lib moves. Fixed — added a test pinning the two to the same paths.lib.
5 systemd/service.j2 Restart=always + no StartLimitBurst → a deterministically-failing prestart loops every 10s forever instead of landing in failed. Pre-existing. Filed separately: #213.

P3 — Low (all fixed or advisory)

# File Issue Route
6 test_prestart.py Hook-ordering assertion compared against the RUNTIME_ENV declaration, not the last append. Fixed — asserts against rindex('>> "$RUNTIME_ENV"').
7 test_prestart.py 600/truncation guard vacuous on the no-web_ui branch. Fixed — added guards to the no-web_ui test.
8 test_builder.py Custom-prestart→hook test used a no-web_ui mock, so HOMARR_URL+hook coexistence was unverified. Fixed — test now enables web_ui and asserts both.
9 prestart.py /var/lib/container-apps/{pkg} literal now constructed in 3 modules (DRY). Pre-existing. Advisory — mitigated by the new consistency test (#4).

Security findings were all confirmations that the design is sound (atomic 600, no path injection, HOSTNAME removal security-neutral) — no action.

Applied fixes (663c818)

  • Tightened the hook-sourced-last assertion to compare against the last runtime.env append.
  • Guarded the no-web_ui branch against a future truncating write.
  • Builder test now exercises a web_ui app (HOMARR_URL + hook coexistence).
  • Added a test pinning the rules.j2 install dir to the prestart source dir.
  • AGENTS.md: HOMARR_URL is web_ui-gated; documented the tmpfs/secret-persistence foot-gun.

Requirements completeness (Plan A)

  • R1–R5, R14 (composable prestart + hook contract + docs) — met in this PR.
  • R11, R12, R13 (marine migration, rollout) — Units 2–3, intentional follow-up after this capability releases. Not expected in this PR.

Recommended follow-ups (not blocking)

  1. Hardening: a build-time validator warning/error when a shipped prestart.sh truncates runtime.env (> "$RUNTIME_ENV" / cat > …runtime.env) — turns the append-only contract from prose into enforcement, closing finding Project Setup and Test Fixtures #1's class. Recorded on Plan A feat: Composable prestart hook (Plan A of 3) #207 Unit 2.
  2. Pre-existing StartLimitBurst restart-loop — filed as fix: generated units lack StartLimit, so a failing prestart loops forever #213.

@mairas mairas merged commit f1f162d into main Jun 15, 2026
4 checks passed
@mairas mairas deleted the feat/composable-prestart-hook branch June 15, 2026 18:28
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.

1 participant