fix: keep stored secrets out of form HTML (A20)

internal/app/handler.go

@@ -739,21 +739,7 @@ func (h *WebHandler) AppAddForm(w http.ResponseWriter, r *http.Request, render R
 		app.Status = StatusDraft
 	}
 
-	envVals := r.Form["env_val"]
-	envTypes := r.Form["env_type"]
-	envList := make([]map[string]interface{}, 0, len(envKeysRaw))
-	for i, k := range envKeysRaw {
-		k = strings.TrimSpace(k)
-		if k == "" || i >= len(envVals) {
-			continue
-		}
-		envList = append(envList, map[string]interface{}{
-			"key":      k,
-			"value":    envVals[i],
-			"isSecret": i < len(envTypes) && envTypes[i] == "secret",
-		})
-	}
-	envJSON, _ := json.Marshal(envList)
+	envJSON, _ := json.Marshal(formEnvVars(r))
 
 	formCtx := func(servers interface{}, errMsg string) map[string]interface{} {
 		return map[string]interface{}{
@@ -856,13 +842,21 @@ func (h *WebHandler) AppEditPage(w http.ResponseWriter, r *http.Request, render
 		}
 	}
 
-	secretsJSON, _ := json.Marshal(secrets)
+	// Secret values stay on the server: the form gets them masked (empty, orig = key) and keeps them unless changed.
+	envList := make([]formEnvVar, 0, len(secrets))
+	for _, s := range secrets {
+		ev := formEnvVar{Key: s.Key, Value: s.Value, IsSecret: s.IsSecret}
+		if s.IsSecret {
+			ev.Value, ev.Masked, ev.Orig = "", true, s.Key
+		}
+		envList = append(envList, ev)
+	}
+	secretsJSON, _ := json.Marshal(envList)
 
 	render(w, r, http.StatusOK, "apps_add.html", map[string]interface{}{
 		"Title":         "Edit " + app.Name,
 		"Servers":       servers,
 		"App":           app,
-		"Secrets":       secrets,
 		"SecretsJSON":   template.JS(secretsJSON),
 		"Files":         files,
 		"IsEdit":        true,
@@ -957,7 +951,12 @@ func (h *WebHandler) AppEditForm(w http.ResponseWriter, r *http.Request, render
 	app.GitRepo = strings.TrimSpace(r.FormValue("git_repo"))
 	app.GitBranch = strings.TrimSpace(r.FormValue("git_branch"))
 	app.AuthUser = strings.TrimSpace(r.FormValue("auth_user"))
-	app.AuthPass = strings.TrimSpace(r.FormValue("auth_pass"))
+	// The form never contains the stored password: empty keeps it, an empty username removes basic auth.
+	if p := strings.TrimSpace(r.FormValue("auth_pass")); p != "" {
+		app.AuthPass = p
+	} else if app.AuthUser == "" {
+		app.AuthPass = ""
+	}
 	app.MemoryLimit = memoryLimit
 	app.CPULimit = cpuLimit
 	app.LogMaxSize = logMaxSize
@@ -966,21 +965,7 @@ func (h *WebHandler) AppEditForm(w http.ResponseWriter, r *http.Request, render
 	app.Ports = ports
 	app.UlimitsNofile = ulimitsNofile
 
-	envVals := r.Form["env_val"]
-	envTypes := r.Form["env_type"]
-	envList := make([]map[string]interface{}, 0, len(envKeysRaw))
-	for i, k := range envKeysRaw {
-		k = strings.TrimSpace(k)
-		if k == "" || i >= len(envVals) {
-			continue
-		}
-		envList = append(envList, map[string]interface{}{
-			"key":      k,
-			"value":    envVals[i],
-			"isSecret": i < len(envTypes) && envTypes[i] == "secret",
-		})
-	}
-	envJSON, _ := json.Marshal(envList)
+	envJSON, _ := json.Marshal(formEnvVars(r))
 
 	editCtx := func(servers interface{}, errMsg string) map[string]interface{} {
 		return map[string]interface{}{
@@ -1291,6 +1276,34 @@ func (h *WebHandler) AppRedeployAll(w http.ResponseWriter, r *http.Request, rend
 	web.Redirect(w, r, "/apps?flash="+url.QueryEscape(fmt.Sprintf("Redeploying %d running apps...", count)), http.StatusSeeOther)
 }
 
+// formEnvVar is one row of the env editor in apps_add.html. Masked: a stored secret sent without its
+// value; Orig is the key it was stored under.
+type formEnvVar struct {
+	Key      string `json:"key"`
+	Value    string `json:"value"`
+	IsSecret bool   `json:"isSecret"`
+	Masked   bool   `json:"masked,omitempty"`
+	Orig     string `json:"orig,omitempty"`
+}
+
+// formEnvVars rebuilds the env rows from a submitted form (re-render after a validation error).
+func formEnvVars(r *http.Request) []formEnvVar {
+	keys, vals, types, origs := r.Form["env_key"], r.Form["env_val"], r.Form["env_type"], r.Form["env_orig"]
+	list := make([]formEnvVar, 0, len(keys))
+	for i, k := range keys {
+		k = strings.TrimSpace(k)
+		if k == "" || i >= len(vals) {
+			continue
+		}
+		ev := formEnvVar{Key: k, Value: vals[i], IsSecret: i < len(types) && types[i] == "secret"}
+		if ev.Value == "" && i < len(origs) && origs[i] != "" {
+			ev.Masked, ev.Orig = true, origs[i]
+		}
+		list = append(list, ev)
+	}
+	return list
+}
+
 func saveFormEnvVars(r *http.Request, svc *Service, appID int64) {
 	existing, _ := svc.ListSecrets(appID)
 	existingMap := make(map[string]AppSecret)
@@ -1301,6 +1314,7 @@ func saveFormEnvVars(r *http.Request, svc *Service, appID int64) {
 	keys := r.Form["env_key"]
 	vals := r.Form["env_val"]
 	types := r.Form["env_type"]
+	origs := r.Form["env_orig"]
 	submitted := make(map[string]bool)
 
 	for i, k := range keys {
@@ -1311,6 +1325,11 @@ func saveFormEnvVars(r *http.Request, svc *Service, appID int64) {
 		submitted[k] = true
 		v := vals[i]
 		isSecret := i < len(types) && types[i] == "secret"
+		if v == "" && i < len(origs) && origs[i] != "" {
+			if ev, ok := existingMap[origs[i]]; ok && ev.IsSecret {
+				v = ev.Value // masked row left unchanged (maybe renamed or made plain)
+			}
+		}
 
 		if isSecret && v == "" {
 			continue

internal/app/validate.go

@@ -52,11 +52,17 @@ func validateForm(r *http.Request, domains []string) error {
 			return err
 		}
 	}
+	seen := map[string]bool{}
 	for _, k := range r.Form["env_key"] {
 		if k = strings.TrimSpace(k); k != "" {
 			if err := ValidEnvKey(k); err != nil {
 				return err
 			}
+			// Two rows with one key would overwrite each other (and a masked secret silently).
+			if seen[k] {
+				return fmt.Errorf("environment variable %s is listed twice", k)
+			}
+			seen[k] = true
 		}
 	}
 	for _, p := range r.Form["file_path"] {

internal/http/secrets_form_test.go

@@ -0,0 +1,124 @@
+package http
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strconv"
+	"strings"
+	"testing"
+
+	"github.com/coderbuzz/dockify/internal/app"
+	"github.com/coderbuzz/dockify/internal/db"
+	"github.com/coderbuzz/dockify/internal/server"
+	"github.com/coderbuzz/dockify/internal/settings"
+)
+
+type noServers struct{}
+
+func (noServers) List() ([]app.ServerInfo, error) { return nil, nil }
+
+// Secrets are never rendered into HTML; the edit form keeps stored values it did not change (A20).
+func TestSecretsNotInFormHTML(t *testing.T) {
+	database, err := db.Open(":memory:")
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { database.Close() })
+	srvRepo := server.NewRepository(database)
+	appSvc := app.NewService(app.NewRepository(database), srvRepo, nil, nil)
+	setSvc := settings.NewService(database, "test")
+	r := NewRouter(server.NewService(srvRepo), appSvc, Render, noServers{}, "admin", "", "", t.TempDir(), func() string { return "" }, settings.NewHandler(setSvc), nil, "test", true, false)
+	do := func(method, path string, form url.Values) *httptest.ResponseRecorder {
+		req := httptest.NewRequest(method, path, strings.NewReader(form.Encode()))
+		if form != nil {
+			req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+		}
+		rec := httptest.NewRecorder()
+		r.ServeHTTP(rec, req)
+		return rec
+	}
+
+	whSecret, _ := setSvc.GetWebhookSecret()
+	if rec := do(http.MethodGet, "/settings", nil); rec.Code != http.StatusOK || strings.Contains(rec.Body.String(), whSecret) {
+		t.Errorf("/settings: %d, contains webhook secret %v", rec.Code, strings.Contains(rec.Body.String(), whSecret))
+	}
+	if rec := do(http.MethodGet, "/api/settings/webhook-secret", nil); !strings.Contains(rec.Body.String(), whSecret) {
+		t.Errorf("API must still return the secret: %d %s", rec.Code, rec.Body)
+	}
+
+	a := &app.App{Name: "web", Compose: "services: {}", Status: app.StatusDraft, AuthUser: "u", AuthPass: "old-pass-123"}
+	if err := appSvc.Create(a); err != nil {
+		t.Fatal(err)
+	}
+	for _, s := range []struct {
+		k, v   string
+		secret bool
+	}{{"API_KEY", "secret-a-123", true}, {"TOKEN", "secret-c-123", true}, {"DROP", "secret-d-123", true}, {"PLAIN", "plain-b", false}} {
+		if err := appSvc.SetSecretWithType(a.ID, s.k, s.v, s.secret); err != nil {
+			t.Fatal(err)
+		}
+	}
+	editPath := "/apps/" + strconv.FormatInt(a.ID, 10) + "/edit"
+	page := do(http.MethodGet, editPath, nil).Body.String()
+	for _, v := range []string{"old-pass-123", "secret-a-123", "secret-c-123", "secret-d-123"} {
+		if strings.Contains(page, v) {
+			t.Errorf("edit page contains %q", v)
+		}
+	}
+	if !strings.Contains(page, "plain-b") || !strings.Contains(page, `placeholder="unchanged"`) {
+		t.Errorf("edit page: plain value or unchanged placeholder missing")
+	}
+
+	// Unchanged password and masked rows: API_KEY renamed, TOKEN made plain, DROP removed, PLAIN kept.
+	form := url.Values{
+		"name": {"web"}, "mode": {"advanced"}, "compose": {"services: {}"}, "draft": {"1"},
+		"auth_user": {"u"}, "auth_pass": {""},
+		"env_key":  {"API_KEY2", "TOKEN", "PLAIN"},
+		"env_val":  {"", "", "plain-b"},
+		"env_type": {"secret", "plain", "plain"},
+		"env_orig": {"API_KEY", "TOKEN", ""},
+	}
+	if rec := do(http.MethodPost, editPath, form); rec.Code != http.StatusSeeOther {
+		t.Fatalf("edit: %d %s", rec.Code, rec.Body)
+	}
+	got, _ := appSvc.Get(a.ID)
+	if got.AuthPass != "old-pass-123" {
+		t.Errorf("AuthPass = %q, want the old one", got.AuthPass)
+	}
+	list, _ := appSvc.ListSecrets(a.ID)
+	m := map[string]app.AppSecret{}
+	for _, s := range list {
+		m[s.Key] = s
+	}
+	if s := m["API_KEY2"]; s.Value != "secret-a-123" || !s.IsSecret {
+		t.Errorf("renamed masked secret: %+v", s)
+	}
+	if s := m["TOKEN"]; s.Value != "secret-c-123" || s.IsSecret {
+		t.Errorf("masked secret made plain: %+v", s)
+	}
+	if _, ok := m["API_KEY"]; ok || len(m) != 3 {
+		t.Errorf("secrets after edit: %+v", m)
+	}
+
+	dup := url.Values{}
+	for k, v := range form {
+		dup[k] = v
+	}
+	dup["env_key"] = []string{"PLAIN", "TOKEN", "PLAIN"}
+	if rec := do(http.MethodPost, editPath, dup); rec.Code != http.StatusBadRequest {
+		t.Errorf("duplicate env key: %d, want 400", rec.Code)
+	}
+
+	form.Set("auth_pass", "new-pass")
+	do(http.MethodPost, editPath, form)
+	if got, _ = appSvc.Get(a.ID); got.AuthPass != "new-pass" {
+		t.Errorf("AuthPass = %q, want new-pass", got.AuthPass)
+	}
+	form.Set("auth_user", "")
+	form.Set("auth_pass", "")
+	do(http.MethodPost, editPath, form)
+	if got, _ = appSvc.Get(a.ID); got.AuthPass != "" || got.AuthUser != "" {
+		t.Errorf("empty username must remove basic auth: %q/%q", got.AuthUser, got.AuthPass)
+	}
+}

internal/http/templates/apps_add.html

@@ -167,7 +167,7 @@ <h3>Basic auth</h3>
       <p class="card-sub">Optional. Protects the app with a username and password.</p>
       <div class="grid">
         <label class="field"><span class="field-label">Username</span><input type="text" name="auth_user" placeholder="admin" autocomplete="off" value="{{if .App}}{{.App.AuthUser}}{{end}}"></label>
-        <label class="field"><span class="field-label">Password</span><input type="password" name="auth_pass" placeholder="password" autocomplete="new-password" value="{{if .App}}{{.App.AuthPass}}{{end}}"></label>
+        <label class="field"><span class="field-label">Password</span><input type="password" name="auth_pass" placeholder="{{if and .IsEdit .App.AuthPass}}unchanged{{else}}password{{end}}" autocomplete="new-password" value="{{if and .App (not .IsEdit)}}{{.App.AuthPass}}{{end}}">{{if .IsEdit}}<small class="field-hint">Leave empty to keep the current password. Clear Username to remove basic auth.</small>{{end}}</label>
       </div>
     </div>
 
@@ -238,7 +238,8 @@ <h3>Basic auth</h3>
 if (Array.isArray(initialEnvVars)) {
   initialEnvVars.forEach(function(ev) {
     var isSecret = ev.isSecret || false;
-    pendingEnvVars.push({key: ev.key || '', value: ev.value || '', isSecret: isSecret});
+    // masked: a stored secret sent without its value; the server keeps it unless the value is changed.
+    pendingEnvVars.push({key: ev.key || '', value: ev.value || '', isSecret: isSecret, masked: !!ev.masked, orig: ev.orig || ''});
   });
 }
 {{end}}
@@ -279,7 +280,7 @@ <h3>Basic auth</h3>
     div.className = 'env-row row-grid row-env';
     var typeLabel = ev.isSecret ? 'Secret' : 'Plain';
     div.innerHTML = '<input type="text" class="env-key-input mono" value="' + escapeAttr(ev.key) + '" placeholder="KEY" autocomplete="off" aria-label="Variable name" onchange="updateEnvKey(' + i + ', this.value)">'
-      + '<textarea class="env-val-input mono" rows="1" placeholder="value" aria-label="Variable value" onchange="updateEnvVal(' + i + ', this.value)">' + escapeHtml(ev.value) + '</textarea>'
+      + '<textarea class="env-val-input mono" rows="1" placeholder="' + (ev.masked ? 'unchanged' : 'value') + '" aria-label="Variable value" onchange="updateEnvVal(' + i + ', this.value)">' + escapeHtml(ev.value) + '</textarea>'
       + '<button type="button" class="btn btn-ghost btn-icon-sm" onclick="toggleEnvType(' + i + ')" title="' + typeLabel + '" aria-label="Toggle secret" aria-pressed="' + (ev.isSecret ? 'true' : 'false') + '">' + (ev.isSecret ? ICON_EYE_OFF : ICON_EYE) + '</button>'
       + '<button type="button" class="btn btn-ghost btn-icon-sm" aria-label="Remove variable" onclick="removeEnvVar(' + i + ')">' + ICON_TRASH + '</button>';
     list.appendChild(div);
@@ -294,6 +295,7 @@ <h3>Basic auth</h3>
 
 function updateEnvVal(index, val) {
   pendingEnvVars[index].value = val;
+  pendingEnvVars[index].masked = false;
   syncEnvHiddenFields();
 }
 
@@ -313,9 +315,14 @@ <h3>Basic auth</h3>
     t.type = 'hidden';
     t.name = 'env_type';
     t.value = ev.isSecret ? 'secret' : 'plain';
+    var o = document.createElement('input');
+    o.type = 'hidden';
+    o.name = 'env_orig';
+    o.value = ev.masked ? ev.orig : '';
     container.appendChild(k);
     container.appendChild(v);
     container.appendChild(t);
+    container.appendChild(o);
   });
 }
 
@@ -348,10 +355,20 @@ <h3>Basic auth</h3>
 }
 
 function openExportModal() {
+  var appId = '{{if .IsEdit}}{{.App.ID}}{{end}}';
+  if (!appId || !pendingEnvVars.some(function(ev){ return ev.masked; })) return showExport({});
+  // Masked secrets are fetched only when the user asks for them.
+  fetch('api/apps/' + appId + '/secrets')
+    .then(function(r){ if (!r.ok) throw new Error(r.status); return r.json(); })
+    .then(function(list){ var m = {}; (list || []).forEach(function(s){ m[s.key] = s.value; }); showExport(m); })
+    .catch(function(){ showToast('Could not load secret values', true); });
+}
+
+function showExport(stored) {
   var lines = [];
   pendingEnvVars.forEach(function(ev) {
     if (ev.key) {
-      lines.push(ev.key + '=' + ev.value);
+      lines.push(ev.key + '=' + (ev.masked ? (stored[ev.orig] || '') : ev.value));
     }
   });
   if (lines.length === 0) {

internal/http/templates/settings.html

@@ -35,7 +35,7 @@ <h3>Appearance</h3>
     <h3>Webhook secret</h3>
     <p class="card-sub">Use this secret in GitHub Actions to authenticate webhook deploy notifications.</p>
     <div class="secret-row" id="wh-active">
-      <input type="password" id="wh-secret" value="{{.WebhookSecret}}" readonly aria-label="Webhook secret">
+      <input type="password" id="wh-secret" value="" placeholder="••••••••••••••••" readonly aria-label="Webhook secret">
       <button class="btn btn-ghost btn-sm" onclick="toggleSecret()" id="toggle-btn">Show</button>
       <button class="btn btn-ghost btn-sm" onclick="copySecret()" id="copy-btn">Copy</button>
       <button class="btn btn-destructive-outline btn-sm" onclick="openSheet('roll-confirm')">Roll secret</button>
@@ -70,14 +70,26 @@ <h3>Import</h3>
 <script>
 document.getElementById('roll-confirm-ok').addEventListener('click', function () { rollSecret(); });
 var visible = false;
+// The secret is not in the page: load it from the API on the first Show or Copy.
+function withSecret(fn) {
+  var el = document.getElementById('wh-secret');
+  if (el.value) return fn(el.value);
+  fetch('api/settings/webhook-secret')
+    .then(function (r) { if (!r.ok) throw new Error(r.status); return r.json(); })
+    .then(function (d) { el.value = d.secret; fn(d.secret); })
+    .catch(function () { showToast('Could not load the webhook secret', true); });
+}
 function toggleSecret() {
   var el = document.getElementById('wh-secret');
   var btn = document.getElementById('toggle-btn');
-  visible = !visible;
-  el.type = visible ? 'text' : 'password';
-  btn.textContent = visible ? 'Hide' : 'Show';
+  if (visible) {
+    visible = false; el.type = 'password'; btn.textContent = 'Show';
+    return;
+  }
+  withSecret(function () { visible = true; el.type = 'text'; btn.textContent = 'Hide'; });
 }
-function copySecret() {
+function copySecret() { withSecret(copyLoadedSecret); }
+function copyLoadedSecret() {
   var el = document.getElementById('wh-secret');
   var btn = document.getElementById('copy-btn');
   function done(ok) { btn.textContent = ok ? 'Copied' : 'Copy failed'; setTimeout(function () { btn.textContent = 'Copy'; }, 1500); }