27d1165962
gates / gates (push) Successful in 17s
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. The page now carries the non-secret half plus a
boolean; the value comes from POST /apps/<slug>/initial-credentials/reveal, which
RE-READS the container rather than serving a cached copy (caching it in the
handler would put it back in the body one layer in). no-store, CSRF-covered,
logged as an act. Both buttons go through it. A reveal that cannot read the value
SAYS SO rather than returning an empty string that renders as a blank password.
Site two, established before changing. The hidden input is NOT the defect and was
left alone: it fires only pre-deploy, and README §318 documents why the value must
round-trip — the customer notes the generated secrets down and submitting them
back is what makes the saved value the same one they saw. The defect was the
neighbouring READONLY input, which on an ALREADY-DEPLOYED app rendered the secret
into a page with nothing to submit. Fixed by POST /stacks/<name>/auto-field/reveal,
authorised by requiring a type:secret auto-field of that stack. Both directions
pinned.
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', about EMPTINESS. No line in the repo says 'no silent
auto-fill'.
The gate. scripts/secret_in_markup_gate.py, registered in controller_gates.py,
convicts any template expression that names a secret unless allowlisted with a
reason. Its limits are MEASURED and in its docstring: it catches a launder through
a local variable (the assignment names the secret) but is blind to a secret
arriving under a neutral page-data key — verified both ways. That is the shape of
site two, which this gate would NOT have caught. The runtime body assertion covers
all shapes but only 4 of 27 page templates; the other 23 are R-255, filed rather
than glossed. Two nets, different holes, both named.
Correction to v0.207.0's report: HTML comments do NOT ship in the response body
here — html/template strips them, text/template does not. Measured. A red-proof
planting a secret in a comment therefore correctly does not fail.
203 lines
9.2 KiB
Go
203 lines
9.2 KiB
Go
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
|
|
// `<span id="initcred-pw-val" hidden>…</span>` — `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 `<span id="initcred-pw-val" hidden>{{.InitialCreds.Password}}</span>` 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")
|
|
}
|
|
}
|