diff --git a/CHANGELOG.md b/CHANGELOG.md index 6110656..d76de9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,61 @@ +## v0.208.0 — the last two secrets leave the page source, and a gate so there is no fourth (2026-08-08, R-254) — MinAgent 0.127.0 + +v0.207.0 removed a password from one page. The census that fix required found two more sites; this +closes both, and adds a check so the next one is caught rather than searched for. + +### 1. An app's first-login password (R-254 site one) + +`app_info.html` rendered `{{.InitialCreds.Password}}` into a `hidden` span — a **real per-install +credential**, read live out of the running container, in the response body of every render. `hidden` +stops a browser DRAWING it and nothing else. + +The page now carries the non-secret half (username, note) plus a boolean; the value comes from +**`POST /apps//initial-credentials/reveal`**, which **re-reads the container** rather than +serving a cached copy — caching it in the handler would put it straight back in the body one layer in. +`no-store`, CSRF-covered, and **logged as an act**. Both buttons (Megjelenítés *and* Másolás) go +through it; neither keeps the value between presses. + +A reveal can now legitimately fail (container stopped, file deleted after first login) and **says so** +— an empty string would have rendered as a blank password. + +### 2. The deploy form — established before changing (R-254 site two) + +The task named the hidden input. **It is not the defect, and it was left alone:** it fires only on the +PRE-DEPLOY form, and `README §318` documents why the value must round-trip — the customer is shown the +generated secrets so they can note them down, and submitting them back is what makes the saved value +the same one they saw ("no silent re-generation on submit"). A form must carry what it submits. + +**The defect was the neighbouring readonly display input.** On an ALREADY-DEPLOYED app the hidden +input is correctly omitted — nothing is being submitted — yet `` still rendered the secret into a page the customer merely opens. That is fixed by +**`POST /stacks//auto-field/reveal`**, authorised by requiring the field to be a `type: secret` +auto-generated field of *that stack's* catalog metadata. Both directions are pinned by tests: the +deployed page must not carry the value, and the pre-deploy form must still submit it. + +**The premise that this contradicted a repo rule does not hold.** The rule is `CONTEXT.md:2070`, +*"Password fields require explicit input — prevents accidental empty-password deployments"*: it is +about EMPTINESS, not auto-fill. No line anywhere in the repo says "no silent auto-fill". + +### 3. A gate, because three instances in two days is a pattern + +`scripts/secret_in_markup_gate.py` (registered in `controller_gates.py`) reads all 36 templates and +convicts any `{{ … }}` whose expression names a secret, unless allowlisted with a stated reason. + +**Its limits are measured, not estimated, and are in its own docstring.** It catches a launder through +a local variable (the assignment names the secret). It is **blind to a secret arriving under a neutral +page-data key** — `data["Tagline"] = creds.Password` then `{{.AppInfo.Tagline}}` passes it cleanly, +verified both ways. That is the shape of site two, which this gate would NOT have caught. + +The complementary net is the runtime body assertion, which catches all of them — but needs each page's +data to be constructible, and **only 4 of 27 page templates have that today**. The other 23 have no +runtime coverage: **R-255**, filed rather than glossed. Two nets, different holes, both named. + +### A correction to v0.207.0's report + +It stated that HTML comments ship in the response body. **They do not, here** — this package renders +with `html/template`, which strips comments (measured: `text/template` keeps them, `html/template` +does not). A red-proof that plants a secret in a comment therefore correctly does **not** fail. + ## v0.207.0 — a password stops living in the page source, and two refusals learn to say what to do (2026-08-08, R-249/R-252/R-253) — MinAgent 0.127.0 Three items the fifth walk exposed by passing. None of them touches the recovery path it proved; all diff --git a/CONTEXT.md b/CONTEXT.md index 659bc21..309162a 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -7,7 +7,38 @@ > > Ask Claude Code: "Please update CONTEXT.md with what we did today" -Last updated: 2026-08-08 (v0.207.0 — R-249/R-252/R-253: a secret leaves the page source, two refusals learn to say what to do) +Last updated: 2026-08-08 (v0.208.0 — R-254: the last two secrets leave the page source, and a gate against a fourth) + +> **2026-08-08 — v0.208.0 (R-254). THE RULE, stated so it outlives this session:** +> +> ### A secret is never in a page's response body. It is fetched by an explicit act, and the act is recorded. +> +> Three instances of one pattern shipped in two days, each found by hand: the retrieval passphrase +> (R-249), an app's real first-login password (R-254 site one), and an already-deployed app's generated +> secret field (R-254 site two). Every one was "hidden" with `display:none`, `hidden`, or +> `type="password"` — **instructions a browser honours when DRAWING and nothing else.** The plaintext +> was in the bytes; a `curl` returned it; caches, history, saved pages and screen-shares had it. +> +> **The shape of the fix, now used three times:** the page carries a BOOLEAN; the value comes from a +> **POST** (so CSRF covers it and it is not re-fetchable from history) with **`Cache-Control: +> no-store`**; the reveal is **LOGGED as an act** — reading a value off markup left no trace anywhere, +> which is why nobody can say whether any of these was ever read. **Per-secret endpoints, never one +> generic "reveal any named secret"** — that would turn three narrow exposures into one lever. +> +> **And the test must assert the RAW RESPONSE BODY.** Every test that asked what the customer *sees* +> passed while the bytes carried the secret. That is precisely how this survived three times. +> +> **What is NOT this defect:** a form must carry what it submits. The pre-deploy hidden input round-trips +> a generated secret deliberately (README §318) so the saved value is the one the customer wrote down. +> The defect there was the neighbouring READONLY input on an already-deployed app, where nothing is +> submitted at all. +> +> **The gate:** `scripts/secret_in_markup_gate.py`. Name-based, all 36 templates, **blind to a secret +> arriving under a neutral page-data key** — measured, not assumed. The complementary runtime +> body-assertion covers 4 of 27 page templates; the other 23 are **R-255**. +> +> **A correction to v0.207.0's report:** it said HTML comments ship in the response body. They do not +> here — `html/template` strips them (`text/template` does not). Measured. > **2026-08-08 — v0.207.0 (R-249, R-252, R-253). Three things the fifth walk exposed BY PASSING.** > The walk closed R-201 (both halves) on 2026-08-07; none of the below touches the recovery path it diff --git a/REUSE.md b/REUSE.md index 9d92a08..774c3d6 100644 --- a/REUSE.md +++ b/REUSE.md @@ -127,7 +127,7 @@ | `EncryptFile` / `DecryptFile` / `IsEncryptedFAB` | controller/internal/appexport/crypto.go | password-based file crypto | .fab export bundles | scrypt-derived AES+HMAC keys | | `maskRepoURL` | controller/internal/sync/sync.go | `(url) string` | Logging git URLs | Strips embedded credentials | | `metrics.RedactLine` | controller/internal/metrics/redact.go | `(s string) string` | ANY log line shipped off-box (issue context, log tails) | Masks password/passwd/secret/token/api-key/authorization/bearer values + 64-hex; apply BEFORE the line leaves the box — controller-side redaction is authoritative | -| `settingsRetrievalPasswordRevealHandler` | controller/internal/web/handlers.go | `POST /settings/retrieval-password/reveal` | **THE PATTERN for showing a secret in the UI** — an XHR that returns only the value | **Never template a secret into a page and hide it with CSS.** `display:none` / `hidden` / `type="password"` stop a browser DRAWING the value; the plaintext is still in the response body, so a `curl` of the page returns it, and it reaches caches, history and any screen-share of the source. R-249 shipped exactly that for two months and was found by it landing in a transcript. The page carries a **boolean** (`HasRetrievalPassword`); the value comes from a POST (CSRF-covered, uncacheable) and the reveal is **logged as an act**. `escrow_handlers.go` states the same rule for R. **Test on the RESPONSE BODY** — a test asserting what the customer *sees* cannot see this class at all. **Known live violations: `app_info.html` (per-install app password in a `hidden` span) and `deploy.html` (auto-generated secret in a `value=`) — R-254.** | +| `settingsRetrievalPasswordRevealHandler` | controller/internal/web/handlers.go | `POST /settings/retrieval-password/reveal` | **THE PATTERN for showing a secret in the UI** — an XHR that returns only the value | **Never template a secret into a page and hide it with CSS.** `display:none` / `hidden` / `type="password"` stop a browser DRAWING the value; the plaintext is still in the response body, so a `curl` of the page returns it, and it reaches caches, history and any screen-share of the source. R-249 shipped exactly that for two months and was found by it landing in a transcript. The page carries a **boolean** (`HasRetrievalPassword`); the value comes from a POST (CSRF-covered, uncacheable) and the reveal is **logged as an act**. `escrow_handlers.go` states the same rule for R. **Test on the RESPONSE BODY** — a test asserting what the customer *sees* cannot see this class at all. **Both R-254 sites are now FIXED the same way** — `POST /apps//initial-credentials/reveal` (re-reads the container, never a cached copy) and `POST /stacks//auto-field/reveal` (authorised on the field being a `type: secret` auto-field of that stack). **Per-secret, never one generic reveal-any-named-secret endpoint.** The PRE-DEPLOY hidden input is deliberate and untouched — a form must carry what it submits (README §318). Enforced by `scripts/secret_in_markup_gate.py`, whose measured blind spot (a secret under a neutral page-data key) is in its docstring; runtime body-assertion covers 4 of 27 pages — R-255. | ### Storage registry + mount detection diff --git a/controller/README.md b/controller/README.md index 398bd97..4fd34ef 100644 --- a/controller/README.md +++ b/controller/README.md @@ -2546,6 +2546,16 @@ During setup wizard drive scan, both current and historical backups are discover Generates `recovery-info.txt` on the system data partition with customer ID, Hub URL, retrieval password, and recovery instructions in Hungarian. Updated on startup and after config changes. Also displayed on the Settings page in a "Vészhelyzeti információk" section. +**No secret is rendered into a page's response body (v0.207.0 + v0.208.0, R-249/R-254).** Three endpoints implement one rule — the page carries a BOOLEAN, the value comes from an explicit authenticated POST with `Cache-Control: no-store`, and the reveal is LOGGED (reading a value off markup left no trace at all): + +| Secret | Endpoint | Read from | +|---|---|---| +| retrieval passphrase | `POST /settings/retrieval-password/reveal` | settings | +| an app's generated first-login password | `POST /apps//initial-credentials/reveal` | **live from the container** — never a cached copy | +| an already-deployed app's auto-generated secret field | `POST /stacks//auto-field/reveal` | the decrypted `app.yaml`; authorised by requiring a `type: secret` auto-field of that stack | + +They are deliberately **per-secret**, not one generic "reveal any named secret" endpoint — that would turn three narrow exposures into one lever with a parameter. **The PRE-DEPLOY hidden input is untouched and deliberate:** a form must carry what it submits (see §318 below). `scripts/secret_in_markup_gate.py` (in `controller_gates.py`) enforces the rule over all templates; its measured blind spot — a secret arriving under a neutral page-data key — is in its docstring. + **The retrieval passphrase is NOT rendered into that page (v0.207.0, R-249).** `securityPageData` passes only `HasRetrievalPassword` (a boolean), and the value is fetched by an explicit act: **`POST /settings/retrieval-password/reveal`** → `{"ok":true,"data":{"password":"…"}}`, behind RequireAuth + CsrfProtect like every other POST, `Cache-Control: no-store`, and logged (`retrieval passphrase revealed via the security page from ` — the value is never logged). Until v0.207.0 the page rendered the plaintext into a `display:none` span, so any fetch of the page returned it; the toggle was cosmetic. This follows the rule `escrow_handlers.go` already states for the recovery code: a secret is revealed by an XHR, never templated server-side into HTML. ### 11. Disaster Recovery diff --git a/controller/internal/web/app_initcreds_exposure_test.go b/controller/internal/web/app_initcreds_exposure_test.go new file mode 100644 index 0000000..764bced --- /dev/null +++ b/controller/internal/web/app_initcreds_exposure_test.go @@ -0,0 +1,202 @@ +package web + +import ( + "bytes" + "io" + "log" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/config" + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" + "gitea.dooplex.hu/admin/felhom-controller/internal/stacks" +) + +// R-254 site one — AN APP'S GENERATED FIRST-LOGIN PASSWORD MUST NOT BE IN THE RESPONSE BODY. +// +// The third instance of one pattern in two days (R-249 was the retrieval passphrase; this is a real +// per-install app credential, read live out of the running container). The markup rendered it into +// `` — `hidden` stops a browser DRAWING the value and +// nothing else, so a `curl` of an app's info page returned it in plaintext. +// +// These assert the RAW BODY. A test that asks what the customer *sees* passes on this defect, which +// is exactly how it survived three times. + +const testAppPassword = "TESTONLY-app-initial-pw-7Kq2mZ" + +// renderAppInfo drives the REAL template with the page data shape the handler produces, and returns +// the bytes a browser would receive. +func renderAppInfo(t *testing.T, creds *stacks.ExtractedCreds, hasPassword bool) string { + t.Helper() + s := securityHarness(t) + s.loadTemplates() + data := map[string]interface{}{ + "Page": "stacks", "Title": "Teszt app", "Domain": "example.hu", + "Stack": stacks.Stack{Name: "crafty", Deployed: true, State: "running"}, + "Meta": stacks.Metadata{DisplayName: "Crafty", Slug: "crafty", Category: "media"}, + "AppInfo": stacks.AppInfo{Tagline: "teszt"}, + // The credentials card lives inside {{if .HasAppInfo}} — without this the card never renders + // and every assertion below would pass for the wrong reason. + "HasAppInfo": true, + } + if creds != nil { + data["InitialCreds"] = creds + data["InitialCredsHasPassword"] = hasPassword + } + var buf bytes.Buffer + if err := s.tmpl.ExecuteTemplate(&buf, "app_info", data); err != nil { + t.Fatalf("render app_info: %v", err) + } + return buf.String() +} + +// ── SCENARIO A — the app password is not in the page ──────────────────────────────────────────── + +// RED-PROOF: put `` back into +// app_info.html AND restore `data["InitialCreds"] = creds` in the handler — this fails on the first +// assertion, showing the plaintext returning to the body. That is the defect, reproduced. +// +// NOTE the second assertion, and a CORRECTION to what v0.207.0's report claimed. It said HTML +// comments ship in the response body. **They do not, here:** this package renders with +// `html/template` (server.go), which STRIPS comments — measured: text/template keeps them, +// html/template does not. So a comment cannot leak a secret, and a red-proof planting one in a +// comment correctly does NOT fail. The assertion is kept because it catches the real regression — +// the hidden element itself coming back into the markup. +func TestAppInfoPage_DoesNotContainTheInitialPassword(t *testing.T) { + html := renderAppInfo(t, &stacks.ExtractedCreds{ + Available: true, Username: "admin", Password: testAppPassword, + }, true) + + if strings.Contains(html, testAppPassword) { + t.Error("R-254: the app's first-login password is in the response body of its info page — " + + "a fetch of this page returns a real per-install credential, and the reveal button only " + + "stops a browser DRAWING it") + } + if strings.Contains(html, "initcred-pw-val") { + t.Error("the old hidden-value element is back in the markup") + } + // The feature must survive: the fix removes the VALUE, not the customer's access (Scenario B). + if !strings.Contains(html, "Kezdeti belépési adatok") { + t.Error("the initial-credentials card vanished — the fix must not take the password away " + + "from the person whose app it is") + } + if !strings.Contains(html, "initial-credentials/reveal") { + t.Error("no reveal call rendered, so the customer has no way to obtain the password at all") + } + // The username is NOT a secret and must still render — otherwise the card is useless. + if !strings.Contains(html, "admin") { + t.Error("the username stopped rendering; only the password was supposed to leave the page") + } +} + +// ── SCENARIO E — an app with no generated credentials is unchanged ────────────────────────────── + +// RED-PROOF: make the card unconditional (drop `{{if .InitialCreds}}`) and this fails — an app that +// has no generated credential grows a reveal control for a password that does not exist. +func TestAppInfoPage_NoCredentialsCard_WhenAppHasNone(t *testing.T) { + html := renderAppInfo(t, nil, false) + + if strings.Contains(html, "Kezdeti belépési adatok") { + t.Error("the initial-credentials card rendered for an app with no generated credentials") + } + if strings.Contains(html, "initial-credentials/reveal") { + t.Error("a reveal control rendered for an app that has no password to reveal") + } +} + +// ── SCENARIO B — the customer can still get it, and C — the act is recorded ───────────────────── + +// credsHarness builds a Server with a REAL stack manager carrying one deployed app whose slug is +// "crafty", plus a capturing logger. Only the container READ is seamed — slug resolution is the +// production path, because a reveal answering for the wrong app is the failure mode that matters. +func credsHarness(t *testing.T) (*Server, *bytes.Buffer) { + t.Helper() + dir := t.TempDir() + buf := &bytes.Buffer{} + lg := log.New(io.MultiWriter(buf), "", 0) + cfg := config.Default() + cfg.Customer.Domain = "example.hu" + cfg.Paths.StacksDir = filepath.Join(dir, "stacks") + cfg.Paths.DataDir = filepath.Join(dir, "data") + cfg.Web.SessionSecret = "test-session-secret-abcdef" + + sd := filepath.Join(cfg.Paths.StacksDir, "crafty") + if err := os.MkdirAll(sd, 0o755); err != nil { + t.Fatal(err) + } + os.WriteFile(filepath.Join(sd, ".felhom.yml"), []byte("display_name: Crafty\nslug: crafty\n"), 0o644) + os.WriteFile(filepath.Join(sd, "docker-compose.yml"), []byte("services: {}\n"), 0o644) + os.WriteFile(filepath.Join(sd, "app.yaml"), []byte("deployed: true\n"), 0o644) + + sett, err := settings.Load(filepath.Join(dir, "settings.json"), lg) + if err != nil { + t.Fatal(err) + } + mgr, err := stacks.NewManager(cfg, lg) + if err != nil { + t.Fatal(err) + } + // Discovery is a separate step — the constructor only prepares the directory. + _ = mgr.ScanStacks() // container-status refresh fails on a docker-less host; discovery is enough + if _, ok := mgr.GetStack("crafty"); !ok { + t.Fatal("crafty not discovered by ScanStacks — the fixture would prove nothing") + } + return &Server{cfg: cfg, settings: sett, stackMgr: mgr, logger: lg, version: "test"}, buf +} + +// RED-PROOF for B: delete the route case from server.go (or make the handler always 404) and the +// customer is shown unable to reach their own app password. +// RED-PROOF for C: delete the s.logger.Printf line and the "recorded" assertion fails. +func TestAppInitialCredsReveal_ReturnsThePasswordAndRecordsTheAct(t *testing.T) { + s, logBuf := credsHarness(t) + s.initialCredsFn = func(name string) (*stacks.ExtractedCreds, error) { + return &stacks.ExtractedCreds{Available: true, Username: "admin", Password: testAppPassword}, nil + } + rr := httptest.NewRecorder() + s.appInitialCredsRevealHandler(rr, httptest.NewRequest("POST", "/apps/crafty/initial-credentials/reveal", nil), "crafty") + + if rr.Code != 200 { + t.Fatalf("reveal returned %d, want 200 — the customer cannot get their own app password", rr.Code) + } + if !strings.Contains(rr.Body.String(), testAppPassword) { + t.Error("the reveal did not return the password — Scenario A's fix must not protect the " + + "secret by taking it from its owner") + } + if got := rr.Header().Get("Cache-Control"); !strings.Contains(got, "no-store") { + t.Errorf("Cache-Control = %q, want no-store — a cached reveal is the same defect one layer down", got) + } + // SCENARIO C — the act is recorded, and the VALUE is not. + logged := logBuf.String() + if !strings.Contains(logged, "initial-credential password revealed") { + t.Error("the reveal was not recorded — a silent read is what the markup allowed, and why " + + "nobody can say whether any of these was ever read") + } + if strings.Contains(logged, testAppPassword) { + t.Error("the password was written to the log") + } +} + +// §7.1 — a reveal that cannot read the value SAYS SO. An empty string would render as a blank +// password and read to the customer as "your password is empty". +func TestAppInitialCredsReveal_SaysWhyWhenUnreadable(t *testing.T) { + s, _ := credsHarness(t) + s.initialCredsFn = func(name string) (*stacks.ExtractedCreds, error) { + return &stacks.ExtractedCreds{Available: false}, nil // container stopped / file gone + } + rr := httptest.NewRecorder() + s.appInitialCredsRevealHandler(rr, httptest.NewRequest("POST", "/apps/crafty/initial-credentials/reveal", nil), "crafty") + + if rr.Code != 404 { + t.Errorf("unreadable reveal returned %d, want 404", rr.Code) + } + body := rr.Body.String() + if !strings.Contains(body, "futnia kell") { + t.Errorf("the refusal does not say WHY it could not be read: %s", body) + } + if strings.Contains(body, `"password"`) { + t.Error("an unreadable reveal still carried a password field") + } +} diff --git a/controller/internal/web/deploy_secret_exposure_test.go b/controller/internal/web/deploy_secret_exposure_test.go new file mode 100644 index 0000000..46de6f3 --- /dev/null +++ b/controller/internal/web/deploy_secret_exposure_test.go @@ -0,0 +1,86 @@ +package web + +import ( + "bytes" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/stacks" +) + +// R-254 site two — WHAT §7.2 ESTABLISHED, PINNED SO IT CANNOT DRIFT BACK. +// +// The deploy page has TWO places a generated secret can appear, and they are NOT the same question: +// +// - the PRE-DEPLOY hidden input — a form must carry what it submits. README §318 documents why: +// the customer is shown the generated secrets so they can note them down, and submitting them +// back is what makes the saved value the SAME one they saw ("no silent re-generation on submit"). +// This is NOT the defect and is deliberately left alone. +// - the READONLY display input on an ALREADY-DEPLOYED app — nothing is being submitted there (the +// hidden input is correctly omitted), yet the value was rendered into the body of a page the +// customer merely opens. That IS R-249's shape, and it is what v0.208.0 fixes. +// +// Both directions are asserted, because "fixed" here means one branch changed and the other did not. + +const testDeploySecret = "TESTONLY-generated-db-pw-Xy91" + +func renderDeployPage(t *testing.T, alreadyDeployed bool) string { + t.Helper() + s := securityHarness(t) + s.loadTemplates() + data := map[string]interface{}{ + "Page": "stacks", "Title": "Telepítés", "Domain": "example.hu", + "Stack": stacks.Stack{Name: "vaultwarden", Deployed: alreadyDeployed}, + "Meta": stacks.Metadata{DisplayName: "Vaultwarden", Slug: "vaultwarden"}, + "AlreadyDeployed": alreadyDeployed, + "AutoFields": []stacks.DeployField{ + {EnvVar: "DB_PASSWORD", Label: "Adatbázis jelszó", Type: "secret"}, + }, + "AutoFieldValues": map[string]string{"DB_PASSWORD": testDeploySecret}, + } + var buf bytes.Buffer + if err := s.tmpl.ExecuteTemplate(&buf, "deploy", data); err != nil { + t.Fatalf("render deploy: %v", err) + } + return buf.String() +} + +// RED-PROOF: drop the `{{if $isDeployed}}` branch so the deployed page renders `value="{{$val}}"` +// again — this fails, showing the secret returning to the body of a page with nothing to submit. +func TestDeployPage_DeployedApp_DoesNotCarryTheSecret(t *testing.T) { + html := renderDeployPage(t, true) + + if strings.Contains(html, testDeploySecret) { + t.Error("R-254 site two: an already-deployed app's generated secret is in the response body " + + "of its settings page — nothing there submits it, so there is no form reason for it to " + + "be in the page at all") + } + // Assert the CONTROL, not the URL: the revealAutoField() function ships in the page script on + // both variants, so a substring match on the endpoint path matches the script and would report a + // control that is not there. (This test caught exactly that on itself.) + if !strings.Contains(html, `onclick="revealAutoField('vaultwarden','DB_PASSWORD'`) { + t.Error("no reveal control rendered, so the customer cannot see their own generated secret") + } + // The hidden input must NOT appear on a deployed app — it never did, and that is the asymmetry + // that makes the readonly input indefensible there. + if strings.Contains(html, ``) { + t.Error("the pre-deploy form no longer submits the generated secret — the saved value would " + + "then not be the one the customer was shown (README §318, 'no silent re-generation on submit')") + } + if strings.Contains(html, `onclick="revealAutoField(`) { + t.Error("the deployed-app reveal control leaked onto the pre-deploy form, where the value is " + + "already legitimately present") + } +} diff --git a/controller/internal/web/handlers.go b/controller/internal/web/handlers.go index 54a2230..fb7f8dd 100644 --- a/controller/internal/web/handlers.go +++ b/controller/internal/web/handlers.go @@ -671,11 +671,29 @@ func (s *Server) appDetailHandler(w http.ResponseWriter, r *http.Request, slug s // Initial auto-generated login (e.g. Crafty writes a random admin password to a file at first // boot). Read it live from the container so the customer doesn't have to dig through logs. Only // for deployed apps that declare an initial_credentials spec; hidden when unreadable. + // + // ⚠ R-254 (v0.208.0) — THE PASSWORD DOES NOT GO INTO THE PAGE DATA, AND THAT IS THE WHOLE FIX. + // + // Until v0.208.0 this handed the whole struct to the template, which rendered the password into + // ``. `hidden` is an attribute the browser honours when + // DRAWING; the plaintext was in the response body of every render, so a `curl` of an app's info + // page returned a real per-install credential. Identical in shape to R-249 one page over, and this + // one is an app the customer actually logs into. + // + // What travels now is the non-secret half (username, note) plus a BOOLEAN. The value is fetched by + // POST /apps//initial-credentials/reveal, which re-reads it LIVE from the container — see + // §7.1: caching it here would put it straight back where it started, one layer in. if found.Deployed && found.Meta.InitialCreds != nil { - if creds, err := s.stackMgr.ReadInitialCredentials(found.Name); err != nil { + if creds, err := s.readInitialCreds(found.Name); err != nil { s.logger.Printf("[WARN] [web] initial-creds for %s: %v", found.Name, err) } else if creds != nil && creds.Available { - data["InitialCreds"] = creds + data["InitialCreds"] = &stacks.ExtractedCreds{ + Available: creds.Available, + Username: creds.Username, + Note: creds.Note, + // Password deliberately NOT carried — the reveal endpoint is the only path to it. + } + data["InitialCredsHasPassword"] = strings.TrimSpace(creds.Password) != "" } } @@ -1662,6 +1680,132 @@ func (s *Server) settingsRetrievalPasswordRevealHandler(w http.ResponseWriter, r escrowJSON(w, http.StatusOK, map[string]any{"password": pw}, "") } +// readInitialCreds is the ONE place an app's generated first-login credential is read — the live +// container read, behind a test seam. Both the info page (which takes only the non-secret half) and +// the reveal endpoint (which takes the value) go through here, so they cannot diverge. +func (s *Server) readInitialCreds(stackName string) (*stacks.ExtractedCreds, error) { + if s.initialCredsFn != nil { + return s.initialCredsFn(stackName) + } + return s.stackMgr.ReadInitialCredentials(stackName) +} + +// appAutoFieldRevealHandler — POST /stacks/{name}/auto-field/reveal (v0.208.0, R-254 site two). +// +// WHAT §7.2 ESTABLISHED, AND WHY THIS EXISTS RATHER THAN A CHANGE TO THE HIDDEN INPUT. +// +// The hidden input (`{{if and (not $isDeployed) (eq .Type "secret")}}`) is NOT this defect. It fires +// only on the PRE-DEPLOY form, and README §318 documents why the value must round-trip: the customer +// is shown the generated secrets so they can write them down, and submitting them back is what makes +// the saved value the SAME one they saw ("no silent re-generation on submit"). A form must carry what +// it submits. +// +// The defect is the neighbouring READONLY display input. On an ALREADY-DEPLOYED app the hidden input +// is correctly omitted — nothing is being submitted — yet `` still renders the secret into the body of a page the customer merely opens. That is +// R-249's shape exactly, with no form to justify it. +// +// PER-SECRET, NOT GENERIC: it serves only fields the CATALOG declares `type: secret` on that stack. +// An env var that is not an auto-generated secret field is refused — that check is the authorisation, +// and it is what stops this becoming "read me any value out of any app's config". +func (s *Server) appAutoFieldRevealHandler(w http.ResponseWriter, r *http.Request, stackName string) { + if s.stackMgr == nil { + escrowJSON(w, http.StatusServiceUnavailable, nil, "Az alkalmazáskezelő nem elérhető.") + return + } + stack, ok := s.stackMgr.GetStack(stackName) + if !ok { + escrowJSON(w, http.StatusNotFound, nil, "Ismeretlen alkalmazás.") + return + } + _ = r.ParseForm() + envVar := strings.TrimSpace(r.FormValue("env_var")) + if envVar == "" { + escrowJSON(w, http.StatusBadRequest, nil, "Hiányzó mező.") + return + } + // AUTHORISATION: the field must be an auto-generated SECRET of this stack's catalog metadata. + allowed := false + for _, f := range stack.Meta.AutoGeneratedFields() { + if f.EnvVar == envVar && f.Type == "secret" { + allowed = true + break + } + } + if !allowed { + s.logger.Printf("[WARN] [web] auto-field reveal refused for %s/%s: not an auto-generated secret field", stackName, envVar) + escrowJSON(w, http.StatusForbidden, nil, "Ez a mező nem kérhető le.") + return + } + appCfg := s.stackMgr.LoadAppConfigByName(stackName) + if appCfg == nil { + escrowJSON(w, http.StatusNotFound, nil, "Az alkalmazás beállításai nem olvashatók.") + return + } + val := crypto.DecryptMap(s.encKey, appCfg.Env)[envVar] + if strings.TrimSpace(val) == "" { + w.Header().Set("Cache-Control", "no-store") + escrowJSON(w, http.StatusNotFound, nil, "Ehhez a mezőhöz nincs mentett érték.") + return + } + s.logger.Printf("[INFO] [web] auto-generated secret revealed for %s/%s from %s (value never logged)", stackName, envVar, clientIP(r)) + w.Header().Set("Cache-Control", "no-store") + escrowJSON(w, http.StatusOK, map[string]any{"value": val}, "") +} + +// appInitialCredsRevealHandler — POST /apps/{slug}/initial-credentials/reveal (v0.208.0, R-254). +// +// The ONLY path by which an app's generated first-login password reaches a browser. Same shape as +// v0.207.0's retrieval-password reveal, deliberately: POST (so CsrfProtect covers it and it is not +// re-fetchable from history), `no-store`, and **logged as an act** — reading it off the markup left +// no trace anywhere, which is why nobody can say whether any of these was ever read. +// +// ⚠ PER-SECRET, NOT GENERIC. This serves exactly one kind of value for one app. A single endpoint +// that returned any named secret would be a worse thing than the defect it fixed: it would turn three +// narrow exposures into one lever with a parameter. +// +// §7.1 — IT RE-READS THE CONTAINER, it does not serve a copy the page already had. Caching the value +// in the handler's page data would put it back in the response body one layer in, which is the defect. +// The consequence is that the reveal can legitimately fail (container stopped, file deleted after +// first login) and it SAYS SO — an empty string here would render as a blank password and read as +// "your password is empty". +func (s *Server) appInitialCredsRevealHandler(w http.ResponseWriter, r *http.Request, slug string) { + if s.stackMgr == nil { + escrowJSON(w, http.StatusServiceUnavailable, nil, "Az alkalmazáskezelő nem elérhető.") + return + } + // Resolved EXACTLY as appDetailHandler resolves it — same loop, same field. A second definition + // of "which app is this slug" is how a reveal ends up answering for a different app than the page + // the customer is looking at. + var found *stacks.Stack + for _, stack := range s.stackMgr.GetStacks() { + if stack.Meta.Slug == slug { + found = &stack + break + } + } + if found == nil { + escrowJSON(w, http.StatusNotFound, nil, "Ismeretlen alkalmazás.") + return + } + creds, err := s.readInitialCreds(found.Name) + if err != nil { + // Never swallowed, and never surfaced raw — the error can name a container/path. + s.logger.Printf("[WARN] [web] initial-creds reveal for %s: %v", found.Name, err) + escrowJSON(w, http.StatusBadGateway, nil, "A kezdeti jelszó beolvasása nem sikerült.") + return + } + if creds == nil || !creds.Available || strings.TrimSpace(creds.Password) == "" { + w.Header().Set("Cache-Control", "no-store") + escrowJSON(w, http.StatusNotFound, nil, + "A kezdeti jelszó most nem olvasható ki — az alkalmazásnak futnia kell hozzá, és lehet, hogy a fájlt az első bejelentkezés után már törölték.") + return + } + s.logger.Printf("[INFO] [web] initial-credential password revealed for %s from %s (value never logged)", found.Name, clientIP(r)) + w.Header().Set("Cache-Control", "no-store") + escrowJSON(w, http.StatusOK, map[string]any{"password": creds.Password}, "") +} + func (s *Server) settingsHandler(w http.ResponseWriter, r *http.Request) { s.executeTemplate(w, r, "settings_system", s.systemPageData()) } diff --git a/controller/internal/web/server.go b/controller/internal/web/server.go index c77dd19..f8d0b15 100644 --- a/controller/internal/web/server.go +++ b/controller/internal/web/server.go @@ -74,6 +74,12 @@ type Server struct { agentCliErr error agentCliOnce sync.Once + // initialCredsFn is the R-254 read seam for an app's generated first-login credential. nil → the + // real live container read (stackMgr.ReadInitialCredentials). ONE definition, used by BOTH the + // info page and the reveal endpoint — two ways to read the same secret is how one of them ends up + // caching it back into the page. + initialCredsFn func(stackName string) (*stacks.ExtractedCreds, error) + // Hub push status callback — set via SetHubPushStatus for monitoring page hubPushStatusFn func() HubPushStatusData @@ -561,6 +567,12 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { s.offboxConfirmEscrowHandler(w, r) case path == "/backup/offbox/inject-password" && r.Method == http.MethodPost: s.offboxInjectPasswordHandler(w, r) + // R-254 site two: an already-deployed app's generated secrets are fetched by an explicit act, + // not rendered into the settings page. The PRE-DEPLOY hidden input is untouched and deliberate + // (README §318) — see the handler for what §7.2 established. + case strings.HasPrefix(path, "/stacks/") && strings.HasSuffix(path, "/auto-field/reveal") && r.Method == http.MethodPost: + name := strings.TrimSuffix(strings.TrimPrefix(path, "/stacks/"), "/auto-field/reveal") + s.appAutoFieldRevealHandler(w, r, name) case strings.HasPrefix(path, "/stacks/") && strings.HasSuffix(path, "/export"): name := strings.TrimPrefix(path, "/stacks/") name = strings.TrimSuffix(name, "/export") @@ -604,6 +616,12 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { fmt.Fprint(w, AppPlaceholderSVG) case strings.HasPrefix(path, "/static/assets/"): s.serveAsset(w, r, strings.TrimPrefix(path, "/static/assets/")) + // R-254: the app's generated first-login password is fetched by an explicit authenticated act, + // never templated into the info page. Placed BEFORE the /apps/ catch-all so the more specific + // path wins. POST (not GET) so CsrfProtect covers it and it is not cacheable — see the handler. + case strings.HasPrefix(path, "/apps/") && strings.HasSuffix(path, "/initial-credentials/reveal") && r.Method == http.MethodPost: + slug := strings.TrimSuffix(strings.TrimPrefix(path, "/apps/"), "/initial-credentials/reveal") + s.appInitialCredsRevealHandler(w, r, slug) case strings.HasPrefix(path, "/apps/"): slug := strings.TrimPrefix(path, "/apps/") s.appDetailHandler(w, r, slug) diff --git a/controller/internal/web/templates/app_info.html b/controller/internal/web/templates/app_info.html index ab5852d..1a7534a 100644 --- a/controller/internal/web/templates/app_info.html +++ b/controller/internal/web/templates/app_info.html @@ -180,11 +180,14 @@ function appMigrate(btn,app,label){ {{end}} Jelszó + •••••••••••• - - + + @@ -212,33 +215,53 @@ function appMigrate(btn,app,label){ {{if .InitialCreds}} {{end}} diff --git a/controller/internal/web/templates/deploy.html b/controller/internal/web/templates/deploy.html index 6e92b65..46f4c09 100644 --- a/controller/internal/web/templates/deploy.html +++ b/controller/internal/web/templates/deploy.html @@ -471,6 +471,7 @@

Ezek az értékek a telepítéssel együtt mentésre kerülnek. Jegyezze fel a szükséges jelszavakat!

{{end}} {{$autoValues := .AutoFieldValues}} + {{$stackName := .Stack.Name}} {{$isDeployed := .AlreadyDeployed}} {{range .AutoFields}} {{$val := index $autoValues .EnvVar}} @@ -478,10 +479,21 @@ {{if $val}} {{if eq .Type "secret"}} + {{if $isDeployed}} + +
+ + +
+ {{else}}
+ {{end}} {{else}} {{end}} @@ -743,6 +755,33 @@ document.addEventListener('DOMContentLoaded', function() { if (sel) checkStorageSpace(sel); }); +// R-254 site two: on an already-deployed app the value is NOT in this page — it is fetched on +// demand, and the server records the act. Nothing caches it between presses. +function revealAutoField(stackName, envVar, btn) { + var el = document.getElementById('auto-field-' + envVar); + if (!el) return; + if (btn.dataset.shown === '1') { + el.value = ''; + el.type = 'password'; + btn.dataset.shown = ''; + btn.textContent = 'Megjelenítés'; + return; + } + btn.disabled = true; + fetch('/stacks/' + encodeURIComponent(stackName) + '/auto-field/reveal', { + method: 'POST', + headers: Object.assign({'Content-Type': 'application/x-www-form-urlencoded'}, csrfHeaders()), + credentials: 'same-origin', + body: 'env_var=' + encodeURIComponent(envVar) + }).then(function (r) { return r.json(); }).then(function (j) { + btn.disabled = false; + if (!j.ok) { showAlert(j.error || 'A lekérés nem sikerült.'); return; } + el.value = j.data.value; + el.type = 'text'; + btn.dataset.shown = '1'; + btn.textContent = 'Elrejtés'; + }).catch(function () { btn.disabled = false; showAlert('A lekérés nem sikerült.'); }); +} function toggleAutoField(fieldId, btn) { var el = document.getElementById(fieldId); if (!el) return; diff --git a/controller/scripts/controller_gates.py b/controller/scripts/controller_gates.py index 7fbc3d3..f7d32e2 100644 --- a/controller/scripts/controller_gates.py +++ b/controller/scripts/controller_gates.py @@ -57,6 +57,7 @@ GATES = [ ("app-row-dedup", os.path.join(SCRIPTS, "app_row_dedup_gate.py"), [], True), ("mojibake", os.path.join(SCRIPTS, "mojibake_gate.py"), [], True), ("docker-v", os.path.join(SCRIPTS, "docker_run_volume_path_gate.py"), [], True), + ("secret-markup", os.path.join(SCRIPTS, "secret_in_markup_gate.py"), [], True), ("reuse-refs", SHARED_REUSE, [REPO], True), ("instructions", SHARED_INSTRUCTIONS, [REPO], True), ] diff --git a/controller/scripts/secret_in_markup_gate.py b/controller/scripts/secret_in_markup_gate.py new file mode 100644 index 0000000..cf56f7f --- /dev/null +++ b/controller/scripts/secret_in_markup_gate.py @@ -0,0 +1,121 @@ +#!/usr/bin/env python3 +"""secret_in_markup_gate.py — a secret must never be rendered into a template. + +WHY THIS EXISTS. Three instances of ONE pattern shipped in two days, each found by hand: + + R-249 settings_security.html {{.RetrievalPassword}} inside a display:none span + R-254 app_info.html {{.InitialCreds.Password}} inside a `hidden` span + R-254 deploy.html value="{{$val}}" in a readonly type=password input + +Each was "hidden" by an instruction the browser honours when DRAWING and by nothing else, so the +plaintext sat in the response body of a page the customer merely opened. **Hiding is not containment.** +A pattern found three times is not closed by searching a fourth time; it is closed by a check. + +WHAT THIS GATE DOES. It reads every template and convicts any `{{ … }}` action whose expression names +a secret (password / secret / token / credential / passphrase / apikey), unless that exact expression +is on the ALLOWLIST below with a stated reason. + +⚠ WHAT IT DOES *NOT* DO, STATED PLAINLY SO NOBODY READS IT AS COMPLETE COVERAGE. + + 1. It is NAME-BASED, and the hole was MEASURED rather than guessed at. It catches + `{{.InitialCreds.Password}}`, and it also catches a launder through a local variable, because the + ASSIGNMENT names the secret (`{{$v := .InitialCreds.Password}}` is convicted). What it cannot see + is a secret that arrives under a NEUTRAL PAGE-DATA KEY — `data["Tagline"] = creds.Password` then + `{{.AppInfo.Tagline}}` passes this gate cleanly. Verified both ways during the 2026-08-08 session. + The third instance above (`value="{{$val}}"` inside an `{{if eq .Type "secret"}}` branch) is that + shape: this gate would NOT have caught it. + 2. It reasons about TEMPLATES, not about rendered output. A handler that writes a secret into a + neutrally-named page-data key is invisible to it. + 3. Runtime body-assertion — rendering a page with a sentinel and grepping the response — is the + check that catches all three, and it needs each page's data to be constructible. Four pages have + that today (settings_security, app_info, deploy, backups_restore) and each has its own test; the + other 23 page templates do NOT. Closing that gap is R-255. + +So: this is the cheap layer that would have caught two of the three, plus a per-page runtime assertion +for the pages that can afford one. Together they are not a proof; they are two nets with different +holes, and the holes are named above. +""" +import os +import re +import sys + +HERE = os.path.dirname(os.path.abspath(__file__)) +CTRL = os.path.dirname(HERE) +TPL = os.path.join(CTRL, "internal", "web", "templates") + +SECRETY = re.compile(r"pass(word|phrase)|secret|token|credential|apikey|api_key", re.I) +ACTION = re.compile(r"\{\{-?\s*(.*?)\s*-?\}\}", re.S) + +# Expressions that name a secret but are NOT one, each with the reason it is safe. An entry here is a +# claim someone made; it should be short enough to re-check by eye. +ALLOWLIST = { + # booleans / presence flags — the whole point of the R-249 and R-254 fixes + ".HasRetrievalPassword": "boolean: whether one exists, never the value", + ".InitialCredsHasPassword": "boolean: whether one exists, never the value", + ".SharePasswordSet": "boolean: whether a share password is set", + ".PasswordError": "an error MESSAGE for a failed password change, not a password", + ".MinPassword": "the minimum LENGTH policy number", + # form field names and types, not values + 'eq .Type "password"': "a field TYPE discriminator", + 'eq .Type "secret"': "a field TYPE discriminator", + 'eq .Type "secret_input"': "a field TYPE discriminator", + 'if or .Required (eq .Type "password")': "a field TYPE discriminator", + 'if and (not $isDeployed) (eq .Type "secret")': "guards the PRE-DEPLOY hidden input — a form must " + "carry what it submits (README §318); see R-254 site two", + "define \"launcher_share_password\"": "a template name", + # the CSRF token is not a secret in this sense: it is bound to the session and useless without it, + # and it MUST be in the form for the form to work. + ".CSRFToken": "CSRF token — session-bound, must be in the page for any POST to work", + ".CSRFField": "CSRF token — same", +} + + +def check(path): + convictions = [] + src = open(path, encoding="utf-8").read() + for m in ACTION.finditer(src): + expr = m.group(1).strip() + if not SECRETY.search(expr): + continue + if expr in ALLOWLIST: + continue + # `{{if .X}}` / `{{with .X}}` where .X is allowlisted is the same claim as `.X` + bare = re.sub(r"^(if|with|else if)\s+", "", expr).strip() + if bare in ALLOWLIST: + continue + line = src[: m.start()].count("\n") + 1 + convictions.append((line, expr)) + return convictions + + +def main(): + if not os.path.isdir(TPL): + print("secret-in-markup gate INCONCLUSIVE: template dir not found: %s" % TPL) + return 2 + files = sorted(f for f in os.listdir(TPL) if f.endswith(".html")) + if not files: + print("secret-in-markup gate INCONCLUSIVE: no templates found in %s" % TPL) + return 2 + total = 0 + bad = 0 + for f in files: + total += 1 + for line, expr in check(os.path.join(TPL, f)): + bad += 1 + print(" %s:%d renders a secret-named expression into the markup: {{%s}}" % (f, line, expr)) + if bad: + print() + print("SECRET-IN-MARKUP GATE FAILED: %d expression(s) across %d template(s)." % (bad, total)) + print("A secret must not be in the response body of a page the customer merely opens —") + print("hiding it with `hidden` / display:none / type=password stops it being DRAWN and nothing else.") + print("Fix: carry a BOOLEAN in the page data and fetch the value with an explicit authenticated") + print("POST that sets Cache-Control: no-store and logs the act (see R-249's and R-254's endpoints).") + print("If the expression genuinely is not a secret, add it to ALLOWLIST with the reason.") + return 1 + print("secret-in-markup gate OK — %d templates, no secret-named expression rendered" % total) + print(" (NAME-BASED: blind to a secret arriving under a neutral PAGE-DATA key — see the docstring)") + return 0 + + +if __name__ == "__main__": + sys.exit(main())