docs: close out ADR-0001 — installer + FINDINGS + close #43/#51 (Child C — refs #54)#60
Merged
Merged
Conversation
…d C — refs #54) Wave 2, Child C of ADR-0001. Documentation pass for the auth collapse shipped in PRs #58 (Child A — FastAPI password + session cookies + dual cookie/Bearer middleware) and #59 (Child B — Caddyfile reduction + PUBLIC_PATHS deletion + --no-tls flag + HAL0_PUBLIC_HOST / HAL0_HARNESS_TLS rename). installer/README.md ------------------- Rewrites the auth section to describe the single FastAPI auth layer. Drops every reference to --auth=basic / HAL0_ADMIN_USER / HAL0_ADMIN_PASSWORD / HAL0_HOSTNAME / Caddy basic_auth / htpasswd. Documents the --no-tls flag (FastAPI binds 0.0.0.0:8080, reachable at http://<host>:8080/). Renames HAL0_HOSTNAME to HAL0_PUBLIC_HOST in the env-var table. Adds an "Upgrade notes (pre-v1)" subsection explaining that existing --auth=basic installs lose edge auth on upgrade and the two mitigations — set a password in the wizard, or --no-tls behind your own reverse proxy. Calls out the wizard's password-setup step (POST /api/auth/password, public on first run per Child A). PLAN.md ------- §1 "Auth + reverse proxy" rewritten to reflect the single FastAPI layer and a "Trust posture" subsection: hal0 defaults to open on the LAN; password auth is opt-in via the dashboard wizard; programmatic clients use Bearer tokens unchanged from #29. Drops the Caddy basic_auth / PUBLIC_PATHS prose; narrows Caddy's scope to TLS termination + reverse proxy. §10 (harness flags) renames HAL0_HARNESS_AUTH → HAL0_HARNESS_TLS to match what the harness scripts actually look for. docs/api-errors.md ------------------ Adds a brief note in the 401 section linking to ADR-0001 / PR #58 and naming the new endpoints (POST /api/auth/login, /api/auth/logout, /api/auth/password). The envelope shapes themselves are unchanged. tests/harness/FINDINGS.md ------------------------- Prepends "FIXED BY ARCHITECTURE REMOVAL (ADR-0001)" notes to the three historical entries that the auth collapse renders structurally unrepeatable: §10 (Caddy basic_auth swallows PUBLIC_PATHS — the original #28 critical bug, fixed in PR #49 and now historical because Caddy no longer has matchers or basicauth per PR #59), §16 (basic_auth password unrecoverable post-install — source of the #43 HITL decision, fixed by deletion because credential capture moved into the wizard per PRs #58 + #59), and §21 (/api/metrics/prometheus orphan in PUBLIC_PATHS — fixed by deletion because PUBLIC_PATHS is gone). The original report bodies are preserved verbatim below each note for historical reference. Re-run instructions updated to the new HAL0_HARNESS_TLS knob. README.md, tests/harness/README.md ---------------------------------- Sync the auth-posture summary in the repo root README and the harness opt-in flags table to match the new single-FastAPI model and the HAL0_HARNESS_TLS rename. These weren't called out in the spec but were left stale after Child B; updating them keeps the docs internally consistent. Closes #43 and #51 (per the parent ADR plan). Issue close comments follow the merge via the gh CLI; this PR body is the GH-semantics hook in case the manual close doesn't land. closes #43 closes #51 refs #54 refs #57 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 17, 2026
thinmintdev
added a commit
that referenced
this pull request
May 21, 2026
ADR-0001 (Collapse edge auth into FastAPI) is implemented. Child A (#58 — FastAPI password auth), Child B (#59 — Caddyfile reduction + --no-tls), and Child C (#60 — docs pass) all landed. Flips the header from Proposed → Accepted, records proposal/acceptance dates separately, names the implementing PRs, and appends an Outcome section summarizing what shipped against the original Decision. Adds #28 (the critical basic_auth ordering bug) to the closed-on-land list per tests/harness/FINDINGS.md §10. README.md and installer/README.md were already brought into line with the v1 single-FastAPI-layer reality in PR #60 — no further changes needed there.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Wave 2, Child C of ADR-0001 — the documentation pass for the auth collapse. No code touched; the entire diff is markdown.
Refs the three-PR sequence:
a405cf7) — FastAPI password auth,/api/auth/login//logout//password, session cookie, dual cookie/Bearer middleware.635cf5a) — Caddyfile collapsed to ~42 lines (TLS terminator + reverse proxy only),PUBLIC_PATHSfrozenset deleted,--auth=basicremoved,--no-tlsflag added,HAL0_HOSTNAME → HAL0_PUBLIC_HOSTandHAL0_HARNESS_AUTH → HAL0_HARNESS_TLSrenames, wizard's password-setup step wired.What changed, per file
installer/README.md## Authenticationsection: single FastAPI layer, no edge-auth, password set via dashboard wizard (POST /api/auth/password, public on first run), Bearer tokens for programmatic clients.--no-tls— skips Caddy, FastAPI binds0.0.0.0:8080, reachable athttp://<host>:8080/. Note that--devimplies--no-tls.--auth=basic,HAL0_ADMIN_USER,HAL0_ADMIN_PASSWORD,HAL0_HOSTNAME,HAL0_AUTH_ENABLED, Caddybasic_auth, htpasswd.HAL0_HOSTNAME → HAL0_PUBLIC_HOSTin the env-var table.--auth=basicinstalls lose edge auth on upgrade; mitigation is set a password in the wizard OR--no-tlsand front with your own reverse proxy.git grep -i basic_auth installer/README.md→ no hits.PLAN.mdHAL0_HARNESS_AUTH → HAL0_HARNESS_TLSto match whatscripts/harness.shandinstaller-test.shactually read.PUBLIC_PATHSmentions; remainingedge authmentions are descriptive (referencing the ADR / describing what Caddy no longer does).docs/api-errors.md/api/auth/login,/api/auth/logout,/api/auth/password). Envelope shapes themselves are unchanged — this is just a pointer.tests/harness/FINDINGS.md/api/metrics/prometheusorphan in PUBLIC_PATHS): same treatment. PUBLIC_PATHS itself is gone, so the allowlist-vs-route drift cannot recur.HAL0_HARNESS_TLS.README.md(repo root)installer/README.md. Small consistency fix.tests/harness/README.mdHAL0_HARNESS_TLS. Same rationale asREADME.md: the oldHAL0_HARNESS_AUTHknob doesn't exist in the scripts anymore, so the doc was wrong.Acceptance checks
The ADR (
docs/adr/0001-...md) mentions PUBLIC_PATHS /--auth=basic/ basicauth by design — it's the historical decision record. The spec explicitly says "Do NOT alter the ADR itself." Similarly,installer/install.sh:333is a code comment outside this PR's scope.Issue housekeeping
After this PR merges, the following issues get closed with explanatory comments naming which PR did what:
This PR body includes the GH-semantics
closes #43/closes #51markers as a fallback; the manual close-with-comment is the preferred path (it adds the explanatory text).CI / billing note
Hal0ai org Actions are billing-blocked — CI on this PR fails in ~2s with no logs. Since this is a docs-only PR, the broken CI is not a risk. User will admin-merge.