From 1895a6b528842fdda30a3fcf56df2df90f15c754 Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Sun, 13 Sep 2026 04:14:16 -0400 Subject: [PATCH 1/4] refactor(sections)!: environments plan through the secrets and variables engines; the sealing key is read at apply The secrets engine reads the scope's sealing key from the first sealed PUT's thunk (once per scope), so the four repo families declare the key endpoint execution-phase and environments' nested secrets share the engine; the variables engine serves environments' nested variables too. Both engines take the scope's home noun and change-line suffix and render through the shared prose helpers; the variables engine proves every payload plain. The per-list duplicate rejecters in environments give way to rejectDuplicates with a resource noun. BREAKING CHANGE: check mode no longer reads the sealing key (a malformed key fails the first PUT at apply); the environment secret and variable lines use the engines' wording. docs/upgrading/v2-to-v3.md sections 10-11. --- docs/upgrading/v2-to-v3.md | 24 +- .../actions_secrets/actions_secrets.test.ts | 24 +- .../actions_variables.test.ts | 19 + src/sections/contract/endpoints.ts | 3 +- src/sections/environments/branch-policies.ts | 23 +- .../environments/environments.test.ts | 34 +- src/sections/environments/index.ts | 2 +- src/sections/environments/nested.ts | 344 ++++++------------ src/sections/environments/protection-rules.ts | 23 +- src/sections/shared/repo-secrets.ts | 7 +- src/sections/shared/repo-variables.ts | 15 +- src/sections/shared/secrets-engine.ts | 110 +++--- src/sections/shared/variables-engine.ts | 74 +++- test/sections/registry.test.ts | 6 +- test/sections/secrets-engine.test.ts | 18 +- 15 files changed, 362 insertions(+), 364 deletions(-) diff --git a/docs/upgrading/v2-to-v3.md b/docs/upgrading/v2-to-v3.md index 6b4e778f..0c42491b 100644 --- a/docs/upgrading/v2-to-v3.md +++ b/docs/upgrading/v2-to-v3.md @@ -4,7 +4,7 @@ order: 20 # Upgrading from v2 to v3 -Eighteen breaks (the ninth is for library consumers). One is silent (the fallback), so run `mode: check` before the first v3 apply. The changelog entry for 3.0.0 will carry the release-please footers in the [CHANGELOG](https://github.com/Vivswan/github-settings-as-code/blob/main/CHANGELOG.md). +Twenty breaks (the ninth is for library consumers). One is silent (the fallback), so run `mode: check` before the first v3 apply. The changelog entry for 3.0.0 will carry the release-please footers in the [CHANGELOG](https://github.com/Vivswan/github-settings-as-code/blob/main/CHANGELOG.md). | Break | v2 | v3 | What the old form does now | |---|---|---|---| @@ -26,6 +26,8 @@ Eighteen breaks (the ninth is for library consumers). One is silent (the fallbac | Underscore keys are directives, never notes | An unknown `_note: ...` at the top level was dropped silently | Only `_layering` and `_undeclared` exist; any other underscore key fails validation, on a wrapper and at the top level alike | Validation fails before any section runs, naming the two directives; [section 16](#16-underscore-keys-are-directives-never-notes) | | `teams` takes the `_undeclared` knob | A plain array; an undeclared team was never listed or touched | `teams: {_undeclared: keep, entries: [...]}` accepted, default `keep`; `delete` revokes undeclared direct grants | No error. Every run now lists the repository's teams and notes each undeclared one; snapshots write the wrapper form; [section 17](#17-teams-takes-the-_undeclared-knob) | | `GSAC_RETRY_BASE_MS` | `RETRY_BASE_MS`, undocumented | `GSAC_RETRY_BASE_MS`, in the inputs reference | No error: an unknown environment variable is ignored, so a harness setting the old name waits real seconds; [section 18](#18-gsac_retry_base_ms) | +| The sealing key is read at apply time | Check mode read `GET .../secrets/public-key` and failed on a malformed key | The first sealed PUT reads it at apply | Check mode issues one request fewer per secret family; a malformed key fails at apply; [section 19](#19-the-sealing-key-is-read-at-apply-time) | +| Environment secrets and variables plan through the shared engines | Their own wording | The engines' wording | Only a grep over the output notices; [section 20](#20-environment-secrets-and-variables-plan-through-the-shared-engines) | ## 1. The defaults-file fallback @@ -277,6 +279,26 @@ The plain array form keeps working, and the [undeclared policy](../reference/und The one environment variable of the tool's own, the retry-timing test knob, is `GSAC_RETRY_BASE_MS`; v2 read it as `RETRY_BASE_MS`, undocumented. An environment variable has no channel for a loud error, so the old name is simply ignored: a harness that set `RETRY_BASE_MS=1` to speed a mock run up now waits real seconds until it is renamed. The [inputs reference](../reference/inputs.md#environment-variables) lists it with the other variables a run reads. +## 19. The sealing key is read at apply time + +Every secret family (`actions_secrets`, `dependabot_secrets`, `codespaces_secrets`, `agents_secrets`) now reads `GET .../secrets/public-key` from the first sealed PUT, as environment secrets always did. + +- Check mode issues one request fewer per family and never touches the key endpoint. +- A malformed key fails the first PUT at apply, before its request leaves, where v2 failed the plan in check mode too. +- The cannot-verify note carries the section label and the shared template: `actions_secrets: Actions secret values cannot be read back from GitHub, so check mode cannot verify them, only that each declared secret exists; apply re-seals and rewrites every declared value on every run`. + +## 20. Environment secrets and variables plan through the shared engines + +Their lines are the engines' lines now: + +| Line | v2 | v3 | +|---|---|---| +| A variable delete | `DELETED undeclared variable "X" from environment "prod"` | `DELETED undeclared variable "X" in environment "prod"` | +| Two entries naming one variable or secret | `environments: the "prod" entry declares variables that GitHub treats as the same variable ...` | `environments: the settings file declares entries that name the same variable of the "prod" environment: "a" and "A". Keep exactly one entry per resource` (branch policies and protection rules spell theirs the same way) | +| The secrets cannot-verify note | once per environment that exists | once per environment with declared secrets, the missing environment included, under the `environments[prod].secrets` label | + +Repository variable operations (`actions_variables`, `agents_variables`) gain a describe line in failure prose (`creating Actions variable "X"`); nothing else moves. + ## Order of operations 1. Rename any settings file whose path contains a comma, rename `undeclared` to `_undeclared` in every settings file, and move every other underscore key into a YAML comment; the v2 line accepts the old spellings only, so do all three together with the pin move. diff --git a/src/sections/actions_secrets/actions_secrets.test.ts b/src/sections/actions_secrets/actions_secrets.test.ts index 13ff5d57..87a345ee 100644 --- a/src/sections/actions_secrets/actions_secrets.test.ts +++ b/src/sections/actions_secrets/actions_secrets.test.ts @@ -20,7 +20,7 @@ const LIST = "GET /repos/o/r/actions/secrets?per_page=100&page=1"; const PUBLIC_KEY = "GET /repos/o/r/actions/secrets/public-key"; const KEY_ROUTE = { data: { key_id: "test-key-id", key: MOCK_SECRETS_PUBLIC_KEY } }; const CANNOT_VERIFY = - "Actions secret values cannot be read back from GitHub, so check mode verifies only that each declared secret exists; apply re-seals and rewrites every declared value on each run"; + "actions_secrets: Actions secret values cannot be read back from GitHub, so check mode cannot verify them, only that each declared secret exists; apply re-seals and rewrites every declared value on every run"; type Declared = Parameters[1]; @@ -141,9 +141,9 @@ describe("actions_secrets planning", () => { notes: [CANNOT_VERIFY], drift: [], }); + // The sealing key is an execution-time read, so check mode issues the list alone. expect(api.calls.map((c) => `${c.method} ${c.path}`)).toEqual([ "GET /repos/o/r/actions/secrets?per_page=100&page=1", - "GET /repos/o/r/actions/secrets/public-key", ]); }); @@ -156,9 +156,15 @@ describe("actions_secrets planning", () => { ]); // Nothing at plan time could have resolved a reference: the resolver is first consulted when a thunk runs. const exec = tools({ $A: "value-a", $B: "value-b" }); - const payloads = result.ops.map((op) => - typeof op.payload === "function" ? sealedPayload({ payload: op.payload(exec) }) : null, + const payloads = await Promise.all( + result.ops.map(async (op) => + typeof op.payload === "function" + ? sealedPayload({ payload: await op.payload(exec) }) + : null, + ), ); + // Both thunks share one key read, issued when the first ran. + expect(api.calls.filter((c) => c.path.endsWith("/public-key")).length).toBe(1); expect(exec.lookups).toEqual(["$A", "$B"]); expect(payloads.map((p) => p?.key_id)).toEqual(["test-key-id", "test-key-id"]); expect(payloads.map((p) => unsealSecretValue(p?.encrypted_value ?? ""))).toEqual([ @@ -413,20 +419,24 @@ describe("actions_secrets execution", () => { }); describe("actions_secrets sealing key", () => { - test("an unusable public key fails the plan loudly, so no operation exists to execute", async () => { + test("an unusable public key fails the first PUT before its request leaves; check mode never reads the key", async () => { const api = new MockApi({ [LIST]: listOf(), [PUBLIC_KEY]: { data: { key: 42 } } }); - await expect(plan(api, [{ name: "X", value: "$V" }])).rejects.toThrow( + const planned = await plan(api, [{ name: "X", value: "$V" }]); + expect(planned.ops.map((op) => op.role)).toEqual(["put"]); + await expect(apply(api, [{ name: "X", value: "$V" }], tools({ $V: "v" }))).rejects.toThrow( /no usable \{key_id, key\} pair \(key_id is missing\)/, ); + expect(api.mutations()).toEqual([]); // GitHub requires key_id in the PUT body to route the ciphertext, so an empty one is as unusable as a missing one. const emptyId = new MockApi({ [LIST]: listOf(), [PUBLIC_KEY]: { data: { key_id: "", key: MOCK_SECRETS_PUBLIC_KEY } }, }); - await expect(plan(emptyId, [{ name: "X", value: "$V" }])).rejects.toThrow( + await expect(apply(emptyId, [{ name: "X", value: "$V" }], tools({ $V: "v" }))).rejects.toThrow( /no usable \{key_id, key\} pair \(key_id is empty\)/, ); + expect(emptyId.mutations()).toEqual([]); }); }); diff --git a/src/sections/actions_variables/actions_variables.test.ts b/src/sections/actions_variables/actions_variables.test.ts index 772574d3..945f5759 100644 --- a/src/sections/actions_variables/actions_variables.test.ts +++ b/src/sections/actions_variables/actions_variables.test.ts @@ -99,6 +99,7 @@ describe("actions_variables", () => { 'actions_variables[DEPLOY_REGION].value: declared "eu-west-1" != live "us-east-1"; apply will set the declared value', ], change: 'updated Actions variable "DEPLOY_REGION"', + describe: 'updating Actions variable "DEPLOY_REGION"', }, { role: "create", @@ -107,6 +108,7 @@ describe("actions_variables", () => { "actions_variables[BUILD_MODE]: missing - declared in the settings file but not on the repo; apply will create it", ], change: 'created Actions variable "BUILD_MODE"', + describe: 'creating Actions variable "BUILD_MODE"', }, { role: "remove", @@ -115,6 +117,7 @@ describe("actions_variables", () => { "actions_variables[RETIRED_FLAG]: undeclared - not in the settings file, so apply will DELETE it; add it to the settings file to keep it", ], change: 'DELETED undeclared Actions variable "RETIRED_FLAG"', + describe: 'deleting undeclared Actions variable "RETIRED_FLAG"', }, ], notes: [], @@ -168,6 +171,22 @@ describe("actions_variables", () => { expect(api.calls).toHaveLength(0); }); + test("a passthrough value JSON cannot carry fails the plan at the payload, on a create and on an update alike", async () => { + // The loose shape admits any extra key; the engine proves the body plain before it is planned, so + // NaN (which JSON would turn into null) never reaches the wire as a silent change of value. + const api = new MockApi(listRoute([{ name: "PRESENT", value: "x" }])); + const odd = { name: "NEW", value: "v", extra: Number.NaN }; + await expect(plan(api, [odd as never])).rejects.toThrow( + /a planned payload carries a value JSON cannot carry at extra/, + ); + await expect(plan(api, [{ ...odd, name: "PRESENT" } as never])).rejects.toThrow( + /a planned payload carries a value JSON cannot carry at extra/, + ); + // The control: a plain extra key rides through. + const plain = await plan(api, [{ name: "NEW", value: "v", extra: 42 } as never]); + expect(plain.ops.map((op) => op.payload)).toEqual([{ name: "NEW", value: "v", extra: 42 }]); + }); + test("wrapped _undeclared:keep leaves the undeclared variable as a note, never a DELETE", async () => { const api = new MockApi(listRoute(liveVariables)); const result = await plan(api, { diff --git a/src/sections/contract/endpoints.ts b/src/sections/contract/endpoints.ts index 9cbcca5f..b352b382 100644 --- a/src/sections/contract/endpoints.ts +++ b/src/sections/contract/endpoints.ts @@ -84,7 +84,8 @@ export interface GatedReadDecl extends EndpointDeclFields { readonly accessGrade: "write"; readonly alwaysRewrite?: never; readonly unverifiable?: never; - readonly phase?: never; + /** As on a plain GET: "execution" gates the read behind a thunk's token (the Codespaces sealing key). */ + readonly phase?: "execution"; } interface EndpointDeclFields { diff --git a/src/sections/environments/branch-policies.ts b/src/sections/environments/branch-policies.ts index 7db3156b..4e8ea038 100644 --- a/src/sections/environments/branch-policies.ts +++ b/src/sections/environments/branch-policies.ts @@ -9,6 +9,7 @@ import type { UndeclaredPolicy } from "../../types.js"; import { liveByIdentity, parseLive } from "../contract/live.js"; import { type SectionMeta, undeclaredDrift, undeclaredNote } from "../contract/module.js"; import { hasDrift, plainData } from "../contract/plan.js"; +import { rejectDuplicates } from "../contract/requests.js"; import { ENDPOINTS, type EnvironmentRestOp, type EnvironmentsRestContext } from "./endpoints.js"; import type { NestedPlan } from "./nested.js"; import type { DeploymentBranchPolicyConfig, EnvironmentConfig } from "./schema.js"; @@ -59,23 +60,17 @@ export function livePolicyName(policy: LiveBranchPolicy, envName: string): strin * the zod shape (schema.ts), not here, so it fails before any section writes. */ export function validateBranchPolicies( + section: SectionMeta, env: EnvironmentConfig, entries: readonly DeploymentBranchPolicyConfig[], ): void { - const seen = new Set(); - const duplicates = new Set(); - for (const pattern of entries) { - if (seen.has(pattern.name)) { - duplicates.add(pattern.name); - } - seen.add(pattern.name); - } - if (duplicates.size > 0) { - throw new Error( - `environments: the "${env.name}" entry declares deployment branch polic${duplicates.size === 1 ? "y" : "ies"} ` + - `${[...duplicates].map((name) => `"${name}"`).join(", ")} more than once. Keep exactly one entry per pattern`, - ); - } + rejectDuplicates( + section, + entries, + (pattern) => pattern.name, + (pattern) => pattern.name, + `deployment branch policy of the "${env.name}" environment`, + ); } function createPolicyOp( diff --git a/src/sections/environments/environments.test.ts b/src/sections/environments/environments.test.ts index 636f698c..3dabf665 100644 --- a/src/sections/environments/environments.test.ts +++ b/src/sections/environments/environments.test.ts @@ -99,7 +99,7 @@ describe("environments plan", () => { 'applied environment "prod"', 'created variable "NEW" in environment "prod"', 'updated variable "UPD" in environment "prod"', - 'DELETED undeclared variable "GONE" from environment "prod"', + 'DELETED undeclared variable "GONE" in environment "prod"', ]); // The strip builds a fresh object, so the duplicate pre-pass (which reads env.variables across all entries) never sees a mutated declaration. expect(declared[0]?.variables).toEqual([ @@ -292,7 +292,7 @@ describe("environments variables case-insensitive matching", () => { }, ]), ).rejects.toThrow( - 'environments: the "prod" entry declares variables that GitHub treats as the same variable (names are case-insensitive): "Region" and "REGION". Keep exactly one entry per variable', + 'environments: the settings file declares entries that name the same variable of the "prod" environment: "Region" and "REGION". Keep exactly one entry per resource', ); expect(api.calls).toEqual([]); }); @@ -308,7 +308,7 @@ describe("environments variables undeclared policy", () => { { name: "prod", variables: { _undeclared: "keep", entries: [] } }, ]); expect(result.notes).toEqual([ - 'variable "LEGACY" exists on environment "prod" but is not declared in the settings file; kept under "_undeclared: keep" - add it to the settings file to manage it, or set "_undeclared: delete" to have apply DELETE it', + 'variable "LEGACY" exists on the environment but is not declared in the settings file; kept under "_undeclared: keep" - add it to the settings file to manage it, or set "_undeclared: delete" to have apply DELETE it', ]); expect(api.mutations()).toEqual([]); }); @@ -319,9 +319,7 @@ describe("environments variables undeclared policy", () => { [VARIABLES_LIST]: variablesBody([{ name: "LEGACY", value: "x" }]), }).allowMutations("DELETE /repos/o/r/environments/prod/variables/LEGACY"); const result = await apply(api, [{ name: "prod", variables: [] }]); - expect(result.changes).toEqual([ - 'DELETED undeclared variable "LEGACY" from environment "prod"', - ]); + expect(result.changes).toEqual(['DELETED undeclared variable "LEGACY" in environment "prod"']); }); test("check mode under _undeclared:keep converges (note, not drift)", async () => { @@ -585,7 +583,7 @@ describe("environments nested secrets validation and shape", () => { ], }, ]), - ).rejects.toThrow(/"prod" entry declares secrets .*"token" and "TOKEN"/); + ).rejects.toThrow(/the same secret of the "prod" environment: "token" and "TOKEN"/); expect(api.calls).toEqual([]); }); @@ -811,7 +809,7 @@ describe("environments deployment branch policies validation and shape", () => { await expect( plan(api, [envWithPolicies([{ name: "release/*" }, { name: "release/*", type: "tag" }])]), ).rejects.toThrow( - 'environments: the "prod" entry declares deployment branch policy "release/*" more than once. Keep exactly one entry per pattern', + 'environments: the settings file declares entries that name the same deployment branch policy of the "prod" environment: "release/*" and "release/*". Keep exactly one entry per resource', ); expect(api.calls).toEqual([]); }); @@ -1137,14 +1135,22 @@ describe("environments missing-environment planning across the nested families", ], ] as Array<[string, EnvironmentConfig, string, string, string]>)( "%s: the sub-resource read is skipped, the note says it is unverifiable, and the create is planned", - async (_key, entry, expectedNote, subResourcePath, createDrift) => { + async (key, entry, expectedNote, subResourcePath, createDrift) => { const api = new MockApi({}); const result = await check(api, [entry]); expect(result.drift).toEqual([ "environments[prod]: missing - declared in the settings file but not on the repo; apply will create it", createDrift, ]); - expect(result.notes).toEqual([expectedNote]); + // The secrets engine states once per scope that values never read back, beside the missing-environment note. + expect(result.notes).toEqual( + key === "secrets" + ? [ + expectedNote, + "environments[prod].secrets: prod environment secret values cannot be read back from GitHub, so check mode cannot verify them, only that each declared secret exists; apply re-seals and rewrites every declared value on every run", + ] + : [expectedNote], + ); expect(api.calls.filter((c) => c.path.includes(subResourcePath))).toEqual([]); }, ); @@ -1161,7 +1167,7 @@ describe("environments deployment protection rules validation and shape", () => }, ]), ).rejects.toThrow( - 'environments: the "prod" entry declares the deployment protection rule App "deploy-gate" more than once. Keep exactly one entry per App', + 'environments: the settings file declares entries that name the same deployment protection rule App of the "prod" environment: "deploy-gate" and "deploy-gate". Keep exactly one entry per resource', ); expect(api.calls).toEqual([]); }); @@ -1358,7 +1364,7 @@ describe("environments convergence", () => { 'applied environment "prod"', 'created variable "DEPLOY_REGION" in environment "prod"', 'updated variable "log_level" in environment "prod"', - 'DELETED undeclared variable "GONE" from environment "prod"', + 'DELETED undeclared variable "GONE" in environment "prod"', 'updated secret "DEPLOY_TOKEN" in environment "prod"', 'created deployment branch policy "release/*" in environment "prod"', 'deleted deployment branch policy "v*" in environment "prod" to change its immutable type (branch -> tag)', @@ -1389,7 +1395,7 @@ describe("environments convergence", () => { "PUT /repos/o/r/environments/prod/secrets/DEPLOY_TOKEN", ]); expect(first.notes).toEqual([ - "prod environment secret values cannot be read back from GitHub, so check mode verifies only that each declared secret exists; apply re-seals and rewrites every declared value on each run", + "environments[prod].secrets: prod environment secret values cannot be read back from GitHub, so check mode cannot verify them, only that each declared secret exists; apply re-seals and rewrites every declared value on every run", 'prod environment secret "KEPT" exists on the environment but is not declared in the ' + 'settings file; kept under "_undeclared: keep" - add it to the settings file to manage ' + 'it, or set "_undeclared: delete" to have apply DELETE it (a deleted secret\'s value is ' + @@ -1471,7 +1477,7 @@ describe("environments convergence", () => { [{ role: "putSecret", drift: [], change: 'updated secret "NEW" in environment "staging"' }], ); expect(second.notes).toEqual([ - "staging environment secret values cannot be read back from GitHub, so check mode verifies only that each declared secret exists; apply re-seals and rewrites every declared value on each run", + "environments[staging].secrets: staging environment secret values cannot be read back from GitHub, so check mode cannot verify them, only that each declared secret exists; apply re-seals and rewrites every declared value on every run", ]); }); }); diff --git a/src/sections/environments/index.ts b/src/sections/environments/index.ts index e7eb1a10..8b91b05c 100644 --- a/src/sections/environments/index.ts +++ b/src/sections/environments/index.ts @@ -72,7 +72,7 @@ export const environmentsSection = { // one entry invalidates. for (const env of desired) { for (const key of NESTED_KEYS) { - validateNested(key, env); + validateNested(this, key, env); } } const plan: EnvironmentsPlan = { ops: [], notes: [], drift: [] }; diff --git a/src/sections/environments/nested.ts b/src/sections/environments/nested.ts index 995b835e..d61c2730 100644 --- a/src/sections/environments/nested.ts +++ b/src/sections/environments/nested.ts @@ -1,22 +1,19 @@ import { z } from "zod"; -import { phantomKeys, phantomNote, subsetDiff } from "../../engine/diff.js"; import type { MustBeNever, UndeclaredPolicy, UndeclaredPolicyList } from "../../types.js"; -import { liveByIdentity, parseLive } from "../contract/live.js"; -import { - type EntryOf, - type SectionMeta, - undeclaredDrift, - undeclaredNote, - undeclaredPolicy, -} from "../contract/module.js"; -import { type ExecTools, hasDrift, plainData } from "../contract/plan.js"; +import { parseLive } from "../contract/live.js"; +import { type EntryOf, type SectionMeta, undeclaredPolicy } from "../contract/module.js"; import { LIVE_SECRET_NAMES, - parseSealingKey, - type SealingKey, - secretKey, + planSecrets, + rejectDuplicateSecretNames, + type SecretsPlanScope, } from "../shared/secrets-engine.js"; -import { LiveVariable, variableKey } from "../shared/variables-engine.js"; +import { + LiveVariable, + planVariables, + rejectDuplicateVariableNames, + type VariablesPlanScope, +} from "../shared/variables-engine.js"; import { BRANCH_POLICIES_DEFAULT_POLICY, planBranchPolicies, @@ -87,7 +84,11 @@ interface NestedPlanner { */ missingNote: (envName: string) => string; /** Rejects misdeclared entries for every environment before anything is read or written. */ - validate?: (env: EnvironmentConfig, entries: readonly NestedEntry[]) => void; + validate?: ( + section: SectionMeta, + env: EnvironmentConfig, + entries: readonly NestedEntry[], + ) => void; plan: ( ctx: EnvironmentsRestContext, section: SectionMeta, @@ -102,6 +103,11 @@ interface NestedPlanner { ) => Promise; } +/** The resource noun a nested duplicate is reported under: `variable of the "prod" environment`. */ +function nestedWhat(noun: string, env: EnvironmentConfig): string { + return `${noun} of the "${env.name}" environment`; +} + const NESTED_PLANNERS: { [K in NestedKey]: NestedPlanner } = { variables: { // "delete" like the top-level actions_variables default: variables are @@ -109,8 +115,9 @@ const NESTED_PLANNERS: { [K in NestedKey]: NestedPlanner } = { defaultPolicy: "delete", missingNote: (envName) => `environments[${envName}].variables: not verifiable while the environment is missing; apply will create the environment and reconcile the declared variables`, - validate: (env, entries) => rejectDuplicateVariables(env.name, entries), - plan: planVariables, + validate: (section, env, entries) => + rejectDuplicateVariableNames(section, entries, nestedWhat("variable", env)), + plan: planEnvironmentVariables, }, secrets: { // "keep" like the top-level secret families: a deleted secret's value is @@ -118,7 +125,8 @@ const NESTED_PLANNERS: { [K in NestedKey]: NestedPlanner } = { defaultPolicy: "keep", missingNote: (envName) => `environments[${envName}].secrets: not verifiable while the environment is missing; apply will create the environment and reconcile the declared secrets`, - validate: (env, entries) => rejectDuplicateSecrets(env.name, entries), + validate: (section, env, entries) => + rejectDuplicateSecretNames(section, entries, nestedWhat("secret", env)), plan: planEnvironmentSecrets, }, deployment_branch_policies: { @@ -158,10 +166,14 @@ export function nestedDefaultPolicy(key: NestedKey): UndeclaredPolicy { return NESTED_PLANNERS[key].defaultPolicy; } -export function validateNested(key: K, env: EnvironmentConfig): void { +export function validateNested( + section: SectionMeta, + key: K, + env: EnvironmentConfig, +): void { const declared = env[key]; if (declared !== undefined) { - NESTED_PLANNERS[key].validate?.(env, unwrapNested(key, declared).entries); + NESTED_PLANNERS[key].validate?.(section, env, unwrapNested(key, declared).entries); } } @@ -225,29 +237,6 @@ export function splitEntry(env: EnvironmentConfig): { return { settings: settings as Record, nested, routed }; } -/** Two spellings of one case-insensitive name would fight each other on every run. */ -function rejectDuplicateVariables( - envName: string, - entries: readonly EnvironmentVariableConfig[], -): void { - const seen = new Map(); - const collisions: string[] = []; - for (const variable of entries) { - const key = variableKey(variable.name); - const first = seen.get(key); - if (first !== undefined) { - collisions.push(`"${first}" and "${variable.name}"`); - continue; - } - seen.set(key, variable.name); - } - if (collisions.length > 0) { - throw new Error( - `environments: the "${envName}" entry declares variables that GitHub treats as the same variable (names are case-insensitive): ${collisions.join("; ")}. Keep exactly one entry per variable`, - ); - } -} - /** One environment's live Actions variables. */ export async function listEnvironmentVariables( ctx: EnvironmentsRestContext, @@ -265,7 +254,20 @@ export async function listEnvironmentVariables( ); } -async function planVariables( +/** The words the two engines render one environment's nested lists with. */ +function nestedProse(envName: string, key: "variables" | "secrets", noun: string) { + return { + label: `environments[${envName}].${key}`, + noun, + where: "the environment", + suffix: ` in environment "${envName}"`, + }; +} + +type Op = Extract; + +/** The variables engine over one environment; an environment the plan creates lists nothing and plans every entry as a create. */ +async function planEnvironmentVariables( ctx: EnvironmentsRestContext, section: SectionMeta, envName: string, @@ -274,113 +276,44 @@ async function planVariables( liveEnv: Record | undefined, ): Promise { const params = { environment_name: envName }; - const label = `environments[${envName}].variables`; - const live = liveEnv === undefined ? [] : await listEnvironmentVariables(ctx, section, envName); - const liveByKey = liveByIdentity( - section, - "variable", - live, - (variable) => variableKey(variable.name), - (variable) => variable.name, - ); - const declaredKeys = new Set(entries.map((variable) => variableKey(variable.name))); - const planned: NestedPlan = { ops: [], notes: [] }; - - for (const variable of entries) { - const entryLabel = `${label}[${variable.name}]`; - const existing = liveByKey.get(variableKey(variable.name)); - const { name: _name, value: _value, ...extraKeys } = variable; - if (!existing) { - planned.ops.push({ - role: "createVariable", - params, - payload: plainData({ name: variable.name, value: variable.value, ...extraKeys }), - drift: [ - `${entryLabel}: missing - declared in the settings file but not on the environment; apply will create it`, - ], - change: `created variable "${variable.name}" in environment "${envName}"`, - describe: `creating variable "${variable.name}" in environment "${envName}"`, - }); - continue; - } - // GitHub stores the name uppercased whatever casing the file uses, so the name never drifts; - // only the value and the declared passthrough fields can. - const drift = [ - ...(existing.value === variable.value - ? [] - : [ - `${entryLabel}.value: declared ${JSON.stringify(variable.value)} != live ${JSON.stringify(existing.value)}; apply will set the declared value`, - ]), - ...subsetDiff(extraKeys, existing, entryLabel), - ]; - if (!hasDrift(drift)) { - continue; - } - const phantom = phantomKeys(extraKeys, existing); - if (phantom.length > 0) { - planned.notes.push(phantomNote(entryLabel, phantom, "variable", "this update will re-run")); - } - planned.ops.push({ + const scope: VariablesPlanScope< + Op<"createVariable">, + Op<"updateVariable">, + Op<"removeVariable"> + > = { + ...nestedProse(envName, "variables", "variable"), + list: async () => + liveEnv === undefined ? [] : await listEnvironmentVariables(ctx, section, envName), + create: (write) => ({ + role: "createVariable", + params, + payload: write.payload, + drift: write.drift, + change: write.change, + describe: write.describe, + }), + update: (write) => ({ role: "updateVariable", - params: { ...params, name: existing.name }, - payload: plainData({ value: variable.value, ...extraKeys }), - drift, - change: `updated variable "${variable.name}" in environment "${envName}"`, - describe: `updating variable "${variable.name}" in environment "${envName}"`, - }); - } - - for (const variable of liveByKey.values()) { - if (declaredKeys.has(variableKey(variable.name))) { - continue; - } - if (policy === "keep") { - planned.notes.push( - undeclaredNote({ - subject: `variable "${variable.name}"`, - state: `exists on environment "${envName}" but is not declared`, - action: "DELETE it", - }), - ); - continue; - } - planned.ops.push({ + params: { ...params, name: write.liveName }, + payload: write.payload, + drift: write.drift, + change: write.change, + describe: write.describe, + }), + remove: (deletion) => ({ role: "removeVariable", - params: { ...params, name: variable.name }, - drift: [ - undeclaredDrift(NESTED_PLANNERS.variables.defaultPolicy, { - label: `${label}[${variable.name}]`, - action: "DELETE it", - }), - ], - change: `DELETED undeclared variable "${variable.name}" from environment "${envName}"`, - describe: `deleting undeclared variable "${variable.name}" from environment "${envName}"`, - }); - } - return planned; -} - -/** Two spellings of one case-insensitive name (GitHub stores secret names uppercase) would fight each other on every run. */ -function rejectDuplicateSecrets( - envName: string, - entries: readonly EnvironmentSecretConfig[], -): void { - const seen = new Map(); - const collisions: string[] = []; - for (const secret of entries) { - const key = secretKey(secret.name); - const first = seen.get(key); - if (first !== undefined) { - collisions.push(`"${first}" and "${secret.name}"`); - continue; - } - seen.set(key, secret.name); - } - if (collisions.length > 0) { - throw new Error( - `environments: the "${envName}" entry declares secrets that GitHub treats as the same secret (names are case-insensitive): ${collisions.join("; ")}. Keep exactly one entry per secret`, - ); - } + params: { ...params, name: deletion.name }, + drift: deletion.drift, + change: deletion.change, + describe: deletion.describe, + }), + }; + const planned = await planVariables(section, scope, { + entries, + policy, + defaultPolicy: NESTED_PLANNERS.variables.defaultPolicy, + }); + return { ops: planned.ops, notes: planned.notes }; } /** One environment's live Actions secret names (GitHub never lists values). */ @@ -401,9 +334,9 @@ export async function listEnvironmentSecrets( } /** - * Existence is the only comparable state (values never read back), so every declared secret is a - * sealed PUT. The sealing key is an execution-phase read (endpoints.ts), issued once per environment - * from the first payload thunk that runs; the token it demands is the one the thunk received. + * The secrets engine over one environment. The sealing key is an execution-phase read (endpoints.ts): + * in apply the environment PUT may only just have created the environment the key belongs to, so the + * engine reads it from the first payload thunk that runs, with the token that thunk received. */ async function planEnvironmentSecrets( ctx: EnvironmentsRestContext, @@ -414,87 +347,32 @@ async function planEnvironmentSecrets( liveEnv: Record | undefined, ): Promise { const params = { environment_name: envName }; - const label = `environments[${envName}].secrets`; - const noun = `${envName} environment secret`; - const suffix = ` in environment "${envName}"`; - const live = liveEnv === undefined ? [] : await listEnvironmentSecrets(ctx, section, envName); - // Real GitHub lists names uppercase already; keying by secretKey keeps a differently-cased mock or proxy harmless. - const liveByKey = new Map( - [ - ...liveByIdentity( - section, - noun, - live, - (item) => secretKey(item.name), - (item) => item.name, - ), - ].map(([key, item]) => [key, item.name]), - ); - const declaredKeys = new Set(entries.map((entry) => secretKey(entry.name))); - const planned: NestedPlan = { ops: [], notes: [] }; - - let sealingKey: Promise | undefined; - const readSealingKey = (exec: ExecTools): Promise => { - sealingKey ??= ctx.read.secretsPublicKey - .call(exec, { params, describe: `reading the ${label} sealing key` }) - .then((body) => parseSealingKey(section, { label }, ENDPOINTS.secretsPublicKey, body)); - return sealingKey; - }; - for (const entry of entries) { - const name = secretKey(entry.name); - const exists = liveByKey.has(name); - planned.ops.push({ + const scope: SecretsPlanScope, Op<"removeSecret">> = { + ...nestedProse(envName, "secrets", `${envName} environment secret`), + list: async () => + liveEnv === undefined ? [] : await listEnvironmentSecrets(ctx, section, envName), + publicKey: (exec, describe) => ctx.read.secretsPublicKey.call(exec, { params, describe }), + publicKeyEndpoint: ENDPOINTS.secretsPublicKey, + put: (write) => ({ role: "putSecret", - params: { ...params, secret_name: name }, - payload: async (exec) => { - const plaintext = exec.resolveSecret(entry.value); - return (await readSealingKey(exec)).seal(plaintext); - }, - drift: exists - ? [] - : [ - `${label}[${name}]: missing - declared in the settings file but not on the environment; apply will create it`, - ], - // The listing decides the verb: the PUT's own 201/204 would say the same, but the executor - // deliberately does not surface statuses. - change: `${exists ? "updated" : "created"} secret "${name}"${suffix}`, - describe: `writing secret "${name}"${suffix}`, - }); - } - if (entries.length > 0 && liveEnv !== undefined) { - // One note per environment; an environment the plan creates already carries the - // missing-environment note, which covers its whole list. - planned.notes.push( - `${noun} values cannot be read back from GitHub, so check mode verifies only that each declared secret exists; apply re-seals and rewrites every declared value on each run`, - ); - } - - for (const [key, liveName] of liveByKey) { - if (declaredKeys.has(key)) { - continue; - } - if (policy === "keep") { - planned.notes.push( - undeclaredNote({ - subject: `${noun} "${liveName}"`, - state: "exists on the environment but is not declared", - action: "DELETE it (a deleted secret's value is unrecoverable)", - }), - ); - continue; - } - planned.ops.push({ + params: { ...params, secret_name: write.name }, + payload: write.payload, + drift: write.drift, + change: write.change, + describe: write.describe, + }), + remove: (deletion) => ({ role: "removeSecret", - params: { ...params, secret_name: liveName }, - drift: [ - undeclaredDrift(NESTED_PLANNERS.secrets.defaultPolicy, { - label: `${label}[${liveName}]`, - action: "DELETE it (the value is unrecoverable)", - }), - ], - change: `DELETED undeclared secret "${liveName}"${suffix}`, - describe: `deleting undeclared secret "${liveName}"${suffix}`, - }); - } - return planned; + params: { ...params, secret_name: deletion.name }, + drift: deletion.drift, + change: deletion.change, + describe: deletion.describe, + }), + }; + const planned = await planSecrets(section, scope, { + entries, + policy, + defaultPolicy: NESTED_PLANNERS.secrets.defaultPolicy, + }); + return { ops: planned.ops, notes: planned.notes }; } diff --git a/src/sections/environments/protection-rules.ts b/src/sections/environments/protection-rules.ts index 17de317f..a41f95a2 100644 --- a/src/sections/environments/protection-rules.ts +++ b/src/sections/environments/protection-rules.ts @@ -3,6 +3,7 @@ import type { UndeclaredPolicy } from "../../types.js"; import { liveByIdentity, parseLive } from "../contract/live.js"; import { type SectionMeta, undeclaredDrift, undeclaredNote } from "../contract/module.js"; import type { ExecTools } from "../contract/plan.js"; +import { rejectDuplicates } from "../contract/requests.js"; import { ENDPOINTS, type EnvironmentsRestContext } from "./endpoints.js"; import type { NestedPlan } from "./nested.js"; import type { DeploymentProtectionRuleConfig, EnvironmentConfig } from "./schema.js"; @@ -111,23 +112,17 @@ async function listProtectionRuleApps( } export function validateProtectionRules( + section: SectionMeta, env: EnvironmentConfig, entries: readonly DeploymentProtectionRuleConfig[], ): void { - const seen = new Set(); - const duplicates = new Set(); - for (const rule of entries) { - if (seen.has(rule.app)) { - duplicates.add(rule.app); - } - seen.add(rule.app); - } - if (duplicates.size > 0) { - throw new Error( - `environments: the "${env.name}" entry declares the deployment protection rule App${duplicates.size === 1 ? "" : "s"} ` + - `${[...duplicates].map((app) => `"${app}"`).join(", ")} more than once. Keep exactly one entry per App`, - ); - } + rejectDuplicates( + section, + entries, + (rule) => rule.app, + (rule) => rule.app, + `deployment protection rule App of the "${env.name}" environment`, + ); } /** Every missing slug resolves from one Apps read before the first POST leaves, so an unlisted slug fails before any rule is half-enabled. */ diff --git a/src/sections/shared/repo-secrets.ts b/src/sections/shared/repo-secrets.ts index e2d6e2be..902e9bb0 100644 --- a/src/sections/shared/repo-secrets.ts +++ b/src/sections/shared/repo-secrets.ts @@ -3,7 +3,7 @@ * same four endpoints under a different path segment and differ only in PAT resource, noun, and (Codespaces) * the grade GitHub gates the reads at, so each section module is ONE repoSecretsSection() call. * - * environments section -> consumes ./secrets-engine.ts directly, with nested scopes + * environments section -> plans its nested secrets through ./secrets-engine.ts too, one scope per environment * .github/scripts/changed-sections.ts -> derives this file's smoke fan-out from the import graph */ @@ -93,6 +93,7 @@ type RepoSecretsEndpoints

= { readonly route: `GET /repos/{owner}/{repo}/${P}/secrets/public-key`; readonly statuses: { readonly 200: string }; readonly accessGrade?: "write"; + readonly phase: "execution"; }; readonly put: { readonly route: `PUT /repos/{owner}/{repo}/${P}/secrets/{secret_name}`; @@ -205,10 +206,12 @@ export function repoSecretsSection(family: { // A fine-grained token conceals a denied list as 404; reading it as "no secrets" would be wrong, so it is a denial. primaryRead: { notFound: "denied" }, }, + // Read inside the first sealed PUT's payload thunk (the engine's rule), so check mode never issues it. publicKey: { route: `GET /repos/{owner}/{repo}/${pathSegment}/secrets/public-key`, statuses: { 200: "the sealing public key" }, ...readGrade, + phase: "execution", }, put: { route: `PUT /repos/{owner}/{repo}/${pathSegment}/secrets/{secret_name}`, @@ -239,7 +242,7 @@ export function repoSecretsSection(family: { LIVE_SECRET_NAMES, await ctx.read.list.listAllEnveloped("secrets"), ), - publicKey: (describe) => ctx.read.publicKey.call({ describe }), + publicKey: (exec, describe) => ctx.read.publicKey.call(exec, { describe }), publicKeyEndpoint: wide.publicKey, put: (write) => ({ role: "put", diff --git a/src/sections/shared/repo-variables.ts b/src/sections/shared/repo-variables.ts index cd31981a..9b53259f 100644 --- a/src/sections/shared/repo-variables.ts +++ b/src/sections/shared/repo-variables.ts @@ -3,7 +3,7 @@ * a different path segment and differ only in PAT resource and noun, so each section module is ONE * repoVariablesSection() call. * - * environments section -> consumes ./variables-engine.ts directly, with nested scopes + * environments section -> plans its nested variables through ./variables-engine.ts too, one scope per environment * .github/scripts/changed-sections.ts -> derives this file's smoke fan-out from the import graph */ @@ -28,12 +28,12 @@ import type { SectionPlan, SnapshotContext, } from "../contract/plan.js"; -import { rejectDuplicates } from "../contract/requests.js"; import { knobbed } from "./schema-helpers.js"; import { knobbedSnapshot, projectOntoSchema } from "./snapshot-helpers.js"; import { LiveVariable, planVariables, + rejectDuplicateVariableNames, type VariableEntry, type VariablesPlanScope, variableKey, @@ -186,13 +186,7 @@ export function repoVariablesSection(family: { const plan: SharedPlan = async (ctx, declared) => { const defaultPolicy = defaultUndeclaredPolicy(section); const { policy, entries } = undeclaredPolicy(declared, defaultPolicy); - // Variable names are case-insensitive on GitHub, so two entries differing only in case name one variable. - rejectDuplicates( - section, - entries, - (variable) => variableKey(variable.name), - (variable) => variable.name, - ); + rejectDuplicateVariableNames(section, entries); // Built where the routes are known, so params typecheck ({name} on update/remove). type Op = PlannedOp; const scope: VariablesPlanScope< @@ -214,6 +208,7 @@ export function repoVariablesSection(family: { payload: write.payload, drift: write.drift, change: write.change, + describe: write.describe, }), update: (write) => ({ role: "update", @@ -221,12 +216,14 @@ export function repoVariablesSection(family: { payload: write.payload, drift: write.drift, change: write.change, + describe: write.describe, }), remove: (deletion) => ({ role: "remove", params: { name: deletion.name }, drift: deletion.drift, change: deletion.change, + describe: deletion.describe, }), }; return planVariables(section, scope, { entries, policy, defaultPolicy }); diff --git a/src/sections/shared/secrets-engine.ts b/src/sections/shared/secrets-engine.ts index b7fefb28..ea5ae968 100644 --- a/src/sections/shared/secrets-engine.ts +++ b/src/sections/shared/secrets-engine.ts @@ -1,10 +1,15 @@ -/** Existence reconciliation over route-free scopes: values are never read back, and every declared secret is re-sealed on each apply. */ +/** + * Existence reconciliation over route-free scopes: values are never read back, and every declared secret + * is re-sealed on each apply. The four repo families (./repo-secrets.ts) and the environments section's + * nested secrets (../environments/nested.ts) plan through it. + */ import { z } from "zod"; import type { UndeclaredPolicy } from "../../types.js"; import { type EndpointDecl, endpointPath } from "../contract/endpoints.js"; import { liveByIdentity } from "../contract/live.js"; import { + cannotVerifyNote, type DeclaredSecretValue, type SectionMeta, secretValuesOf, @@ -36,11 +41,16 @@ export type SealedSecretPayload = { type AnyPlannedOp = SectionPlan["ops"][number]; -interface SecretsScopeProse { +/** The words a scope supplies; a nested scope names its home so the lines say where the secret lives. */ +export interface SecretsScopeProse { /** The drift-line prefix, e.g. "actions_secrets" or "environments[prod].secrets". */ label: string; /** The noun for notes ("Actions secret"; a nested scope says "prod environment secret"). */ noun: string; + /** Where the secrets live when "the repo" understates it ("the environment"). */ + where?: string; + /** Appended to change and describe lines (` in environment "prod"`). */ + suffix?: string; } /** The payload thunk resolves and seals only when executed, so the plan carries the `$NAME` reference and nothing derived from a value. */ @@ -49,8 +59,8 @@ interface SealedSecretWrite { readonly name: string; /** What the write is doing, in settings-file terms, for its error prose. */ readonly describe: string; - /** Seals the resolved plaintext against the scope's sealing key at execution time. */ - readonly payload: (exec: ExecTools) => SealedSecretPayload; + /** Reads the scope's sealing key (once per plan) and seals the resolved plaintext, at execution time. */ + readonly payload: (exec: ExecTools) => Promise; /** The missing-secret line, or empty when the name exists (the PUT recurs by declaration). */ readonly drift: readonly string[]; readonly change: string; @@ -70,8 +80,11 @@ export interface SecretsPlanScope Promise; - /** GET the {key_id, key} sealing key for this scope. */ - readonly publicKey: (describe: string) => Promise; + /** + * GET the {key_id, key} sealing key for this scope, at EXECUTION time: an environment's key exists + * only once the PUT that creates the environment has landed, so the read rides the first PUT's thunk. + */ + readonly publicKey: (exec: ExecTools, describe: string) => Promise; /** The declaration behind `publicKey`, named in the prose of a key the endpoint cannot supply. */ readonly publicKeyEndpoint: EndpointDecl; /** The planned sealed PUT; function-valued so a builder demanding an unsupplied facet fails. */ @@ -84,11 +97,7 @@ export function secretKey(name: string): string { return name.toUpperCase(); } -/** - * Each value is labelled with its entry's secret NAME so a validation error can point at it. DEFENSIVE by - * contract: a malformed container returns [] instead of throwing, so the actionable error always comes - * from shape validation, never a TypeError here. - */ +/** Each value is labelled with its entry's secret NAME so a validation error can point at it. */ export function listSecretValues(declared: unknown): DeclaredSecretValue[] { return secretValuesOf(declared, (entry) => { if (typeof entry.value !== "string") { @@ -102,16 +111,21 @@ export function listSecretValues(declared: unknown): DeclaredSecretValue[] { }); } -/** GitHub folds two names equal uppercased into one secret, so the last write would silently win on every run. */ +/** + * GitHub folds two names equal uppercased into one secret, so the last write would silently win on + * every run. `what` names the resource when "

entry" understates it (a nested scope's). + */ export function rejectDuplicateSecretNames( section: SectionMeta, entries: readonly SecretEntry[], + what?: string, ): void { rejectDuplicates( section, entries, (entry) => secretKey(entry.name), (entry) => entry.name, + what, ); } @@ -179,19 +193,19 @@ export function parseSealingKey( }; } -function missingSecretDrift(scope: SecretsScopeProse, name: string): string { - return `${scope.label}[${name}]: missing - declared in the settings file but not on the repo; apply will create it`; -} - /** ONE note per scope (the LFS precedent): values are unverifiable by design. */ -function cannotVerifyNote(scope: SecretsScopeProse): string { - return `${scope.noun} values cannot be read back from GitHub, so check mode verifies only that each declared secret exists; apply re-seals and rewrites every declared value on each run`; +function valuesUnverifiableNote(scope: SecretsScopeProse): string { + return cannotVerifyNote(scope.label, { + why: `${scope.noun} values cannot be read back from GitHub`, + what: "them, only that each declared secret exists", + reasserts: "re-seals and rewrites every declared value", + }); } function undeclaredSecretNote(scope: SecretsScopeProse, liveName: string): string { return undeclaredNote({ subject: `${scope.noun} "${liveName}"`, - state: "exists on the repo but is not declared", + state: `exists on ${scope.where ?? "the repo"} but is not declared`, action: "DELETE it (a deleted secret's value is unrecoverable)", }); } @@ -210,7 +224,7 @@ function undeclaredSecretDrift( /** Uppercase key -> the name as listed (normalizing keeps a differently-cased mock harmless). */ function liveSecretsByKey( section: SectionMeta, - scope: SecretsScopeProse, + scope: SecretsPlanScope, live: readonly LiveSecretName[], ): Map { const byKey = liveByIdentity( @@ -237,33 +251,43 @@ export async function planSecrets> { const { entries, policy, defaultPolicy } = opts; + const suffix = scope.suffix ?? ""; const plan: SectionPlan = { ops: [], notes: [], drift: [] }; const liveByKey = liveSecretsByKey(section, scope, await scope.list()); const declaredKeys = new Set(entries.map((entry) => secretKey(entry.name))); - if (entries.length > 0) { - const sealingKey = parseSealingKey( - section, - scope, - scope.publicKeyEndpoint, - await scope.publicKey(`reading the ${scope.label} sealing key`), + // Read once per scope, by the first payload thunk that runs; the token it demands is the one the thunk received. + let sealingKey: Promise | undefined; + const readSealingKey = (exec: ExecTools): Promise => { + sealingKey ??= scope + .publicKey(exec, `reading the ${scope.label} sealing key`) + .then((body) => parseSealingKey(section, scope, scope.publicKeyEndpoint, body)); + return sealingKey; + }; + for (const entry of entries) { + const name = secretKey(entry.name); + // The listing decides the verb; the executor does not surface the PUT's 201/204. + const exists = liveByKey.has(name); + plan.ops.push( + scope.put({ + name, + describe: `writing secret "${name}"${suffix}`, + payload: async (exec) => { + const plaintext = exec.resolveSecret(entry.value); + return (await readSealingKey(exec)).seal(plaintext); + }, + drift: exists + ? [] + : [ + `${scope.label}[${name}]: missing - declared in the settings file but not on ${scope.where ?? "the repo"}; apply will create it`, + ], + change: `${exists ? "updated" : "created"} secret "${name}"${suffix}`, + }), ); - for (const entry of entries) { - const name = secretKey(entry.name); - // The listing decides the verb; the executor does not surface the PUT's 201/204. - const exists = liveByKey.has(name); - plan.ops.push( - scope.put({ - name, - describe: `writing secret "${name}"`, - payload: (exec) => sealingKey.seal(exec.resolveSecret(entry.value)), - drift: exists ? [] : [missingSecretDrift(scope, name)], - change: `${exists ? "updated" : "created"} secret "${name}"`, - }), - ); - } - plan.notes.push(cannotVerifyNote(scope)); + } + if (entries.length > 0) { + plan.notes.push(valuesUnverifiableNote(scope)); } for (const [key, liveName] of liveByKey) { @@ -276,9 +300,9 @@ export async function planSecrets; type AnyPlannedOp = SectionPlan["ops"][number]; -type PlainPayload = Exclude, (exec: ExecTools) => unknown>; +type PlainPayload = PlainData; /** The index signature types the passthrough fields as plain data, so spreading them into a body needs no cast. */ export interface VariableEntry { @@ -26,17 +36,24 @@ export interface VariableEntry { readonly [key: string]: PlainPayload | undefined; } +/** The words a scope supplies; a nested scope names its home so the lines say where the variable lives. */ interface VariablesScopeProse { /** The drift-line prefix, e.g. "actions_variables" or "environments[prod].variables". */ label: string; /** The noun for notes and change lines ("Actions variable"). */ noun: string; + /** Where the variables live when "the repo" understates it ("the environment"). */ + where?: string; + /** Appended to change and describe lines (` in environment "prod"`). */ + suffix?: string; } interface VariableCreate { readonly payload: PlainPayload; readonly drift: readonly [string]; readonly change: string; + /** What the write is doing, in settings-file terms, for its error prose. */ + readonly describe: string; } interface VariableUpdate { @@ -45,6 +62,7 @@ interface VariableUpdate { readonly payload: PlainPayload; readonly drift: readonly [string, ...string[]]; readonly change: string; + readonly describe: string; } interface VariableDeletion { @@ -52,6 +70,7 @@ interface VariableDeletion { readonly name: string; readonly drift: readonly [string]; readonly change: string; + readonly describe: string; } /** The type parameters are the section's exact PlannedOp arms, so a wrong role fails to compile. */ @@ -68,18 +87,25 @@ export interface VariablesPlanScope< readonly remove: (deletion: VariableDeletion) => Remove; } -function missingVariableDrift(label: string): string { - return `${label}: missing - declared in the settings file but not on the repo; apply will create it`; -} - -function valueDriftLine(label: string, declared: string, live: string): string { - return `${label}.value: declared ${JSON.stringify(declared)} != live ${JSON.stringify(live)}; apply will set the declared value`; +/** Variable names are case-insensitive on GitHub, so two entries differing only in case name one variable. */ +export function rejectDuplicateVariableNames( + section: SectionMeta, + entries: readonly VariableEntry[], + what?: string, +): void { + rejectDuplicates( + section, + entries, + (variable) => variableKey(variable.name), + (variable) => variable.name, + what, + ); } function undeclaredVariableNote(scope: VariablesScopeProse, liveName: string): string { return undeclaredNote({ subject: `${scope.noun} "${liveName}"`, - state: "exists on the repo but is not declared", + state: `exists on ${scope.where ?? "the repo"} but is not declared`, action: "DELETE it", }); } @@ -113,6 +139,7 @@ export async function planVariables< }, ): Promise> { const { entries, policy, defaultPolicy } = opts; + const suffix = scope.suffix ?? ""; const plan: SectionPlan = { ops: [], notes: [], drift: [] }; const liveByKey = liveByIdentity( @@ -131,9 +158,12 @@ export async function planVariables< if (!existing) { plan.ops.push( scope.create({ - payload: { name: variable.name, value: variable.value, ...extraKeys }, - drift: [missingVariableDrift(label)], - change: `created ${scope.noun} "${variable.name}"`, + payload: plainData({ name: variable.name, value: variable.value, ...extraKeys }), + drift: [ + `${label}: missing - declared in the settings file but not on ${scope.where ?? "the repo"}; apply will create it`, + ], + change: `created ${scope.noun} "${variable.name}"${suffix}`, + describe: `creating ${scope.noun} "${variable.name}"${suffix}`, }), ); continue; @@ -142,7 +172,13 @@ export async function planVariables< // GitHub stores the name uppercased whatever casing the file uses, so the live name never drifts; only the value and passthrough fields can. const [first, ...rest] = [ ...(existing.value !== variable.value - ? [valueDriftLine(label, variable.value, existing.value)] + ? [ + valueDrift( + `${label}.value`, + JSON.stringify(variable.value), + JSON.stringify(existing.value), + ), + ] : []), ...subsetDiff(extraKeys, existing, label), ]; @@ -156,9 +192,10 @@ export async function planVariables< plan.ops.push( scope.update({ liveName: existing.name, - payload: { value: variable.value, ...extraKeys }, + payload: plainData({ value: variable.value, ...extraKeys }), drift: [first, ...rest], - change: `updated ${scope.noun} "${variable.name}"`, + change: `updated ${scope.noun} "${variable.name}"${suffix}`, + describe: `updating ${scope.noun} "${variable.name}"${suffix}`, }), ); } @@ -174,7 +211,8 @@ export async function planVariables< scope.remove({ name: variable.name, drift: [undeclaredVariableDrift(scope, defaultPolicy, variable.name)], - change: `DELETED undeclared ${scope.noun} "${variable.name}"`, + change: `DELETED undeclared ${scope.noun} "${variable.name}"${suffix}`, + describe: `deleting undeclared ${scope.noun} "${variable.name}"${suffix}`, }), ); } diff --git a/test/sections/registry.test.ts b/test/sections/registry.test.ts index 978d9a20..fe0db5b1 100644 --- a/test/sections/registry.test.ts +++ b/test/sections/registry.test.ts @@ -1274,7 +1274,7 @@ describe("handler contracts", () => { ); }); - test("the execution-phase reads are exactly the thunk-issued lookups: the branches ids and the environment sealing key", () => { + test("the execution-phase reads are exactly the thunk-issued lookups: the branches ids and every sealing key", () => { // Check mode never issues these (the e2e mock fails a check-mode arrival), so a read that moves in or out of this list changes what a // read-only token meets in check mode. const byLateReads = SECTIONS.flatMap((section) => { @@ -1286,6 +1286,10 @@ describe("handler contracts", () => { expect(Object.fromEntries(byLateReads)).toEqual({ branches: ["appLookup", "repoLookup", "actorUser", "actorTeam"], environments: ["secretsPublicKey", "listProtectionRuleApps"], + actions_secrets: ["publicKey"], + dependabot_secrets: ["publicKey"], + codespaces_secrets: ["publicKey"], + agents_secrets: ["publicKey"], }); }); }); diff --git a/test/sections/secrets-engine.test.ts b/test/sections/secrets-engine.test.ts index d70dbc63..33b25c98 100644 --- a/test/sections/secrets-engine.test.ts +++ b/test/sections/secrets-engine.test.ts @@ -55,7 +55,7 @@ function fabricatedPlanScope(live: string[], reads: string[]): SecretsPlanScope< return live.map((name) => ({ name })); }, publicKeyEndpoint: PUBLIC_KEY_ENDPOINT, - publicKey: async (describe) => { + publicKey: async (_exec, describe) => { reads.push(describe); return KEY_DATA; }, @@ -156,7 +156,7 @@ describe("secretKey and duplicates", () => { }); describe("planSecrets and the execution-time resolver", () => { - test("each PUT's thunk seals its OWN entry's resolved value, uppercasing the name; planning resolves nothing", async () => { + test("each PUT's thunk seals its OWN entry's resolved value, uppercasing the name; planning reads the list alone and the key once at execution", async () => { await mockSodiumReady(); const reads: string[] = []; const plan = await planSecrets(section, fabricatedPlanScope([], reads), { @@ -167,8 +167,8 @@ describe("planSecrets and the execution-time resolver", () => { policy: "keep", defaultPolicy: "keep", }); - // The sealing key is read once at plan time, after the list. - expect(reads).toEqual(["list", "reading the actions_secrets sealing key"]); + // The sealing key is an execution-time read: check mode never issues it. + expect(reads).toEqual(["list"]); expect(plan.ops.map((op) => op.params)).toEqual([ { secret_name: "FIRST" }, { secret_name: "SECOND" }, @@ -180,9 +180,15 @@ describe("planSecrets and the execution-time resolver", () => { return resolver({ $ONE: "plain-1", $TWO: "plain-2" })(reference); }, }; - const sealed = plan.ops.map((op) => - typeof op.payload === "function" ? (op.payload(exec) as SealedSecretPayload) : null, + const sealed = await Promise.all( + plan.ops.map((op) => + typeof op.payload === "function" + ? (op.payload(exec) as Promise) + : Promise.resolve(null), + ), ); + // Both thunks share ONE key read. + expect(reads).toEqual(["list", "reading the actions_secrets sealing key"]); expect(lookups).toEqual(["$ONE", "$TWO"]); expect(sealed.map((p) => p?.key_id)).toEqual(["key-1", "key-1"]); expect(sealed.map((p) => unsealSecretValue(p?.encrypted_value ?? ""))).toEqual([ From 5cf9b2a29f8f4cecc848dcbbfeea7e45f9f13b3f Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Sun, 13 Sep 2026 07:25:49 -0400 Subject: [PATCH 2/4] fix(environments): a kept undeclared variable note names its environment again --- docs/upgrading/v2-to-v3.md | 1 + src/sections/environments/environments.test.ts | 6 +++--- src/sections/environments/nested.ts | 8 ++++++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/docs/upgrading/v2-to-v3.md b/docs/upgrading/v2-to-v3.md index 0c42491b..84bd1936 100644 --- a/docs/upgrading/v2-to-v3.md +++ b/docs/upgrading/v2-to-v3.md @@ -293,6 +293,7 @@ Their lines are the engines' lines now: | Line | v2 | v3 | |---|---|---| +| A missing declared variable | `environments[prod].variables[X]: missing - declared in the settings file but not on the environment; ...` | `... but not on environment "prod"; ...` | | A variable delete | `DELETED undeclared variable "X" from environment "prod"` | `DELETED undeclared variable "X" in environment "prod"` | | Two entries naming one variable or secret | `environments: the "prod" entry declares variables that GitHub treats as the same variable ...` | `environments: the settings file declares entries that name the same variable of the "prod" environment: "a" and "A". Keep exactly one entry per resource` (branch policies and protection rules spell theirs the same way) | | The secrets cannot-verify note | once per environment that exists | once per environment with declared secrets, the missing environment included, under the `environments[prod].secrets` label | diff --git a/src/sections/environments/environments.test.ts b/src/sections/environments/environments.test.ts index 3dabf665..bff28c50 100644 --- a/src/sections/environments/environments.test.ts +++ b/src/sections/environments/environments.test.ts @@ -251,7 +251,7 @@ describe("environments variables check mode", () => { }); const result = await check(api, [{ name: "prod", variables: [{ name: "A", value: "1" }] }]); expect(result.drift).toEqual([ - "environments[prod].variables[A]: missing - declared in the settings file but not on the environment; apply will create it", + 'environments[prod].variables[A]: missing - declared in the settings file but not on environment "prod"; apply will create it', ]); }); }); @@ -308,7 +308,7 @@ describe("environments variables undeclared policy", () => { { name: "prod", variables: { _undeclared: "keep", entries: [] } }, ]); expect(result.notes).toEqual([ - 'variable "LEGACY" exists on the environment but is not declared in the settings file; kept under "_undeclared: keep" - add it to the settings file to manage it, or set "_undeclared: delete" to have apply DELETE it', + 'variable "LEGACY" exists on environment "prod" but is not declared in the settings file; kept under "_undeclared: keep" - add it to the settings file to manage it, or set "_undeclared: delete" to have apply DELETE it', ]); expect(api.mutations()).toEqual([]); }); @@ -1110,7 +1110,7 @@ describe("environments missing-environment planning across the nested families", { name: "prod", variables: [{ name: "A", value: "1" }] }, "environments[prod].variables: not verifiable while the environment is missing; apply will create the environment and reconcile the declared variables", "/variables", - "environments[prod].variables[A]: missing - declared in the settings file but not on the environment; apply will create it", + 'environments[prod].variables[A]: missing - declared in the settings file but not on environment "prod"; apply will create it', ], [ "secrets", diff --git a/src/sections/environments/nested.ts b/src/sections/environments/nested.ts index d61c2730..b112e1ea 100644 --- a/src/sections/environments/nested.ts +++ b/src/sections/environments/nested.ts @@ -254,12 +254,16 @@ export async function listEnvironmentVariables( ); } -/** The words the two engines render one environment's nested lists with. */ +/** + * The words the two engines render one environment's nested lists with. A variable's kept note names + * the environment (two environments holding the same undeclared name would otherwise emit one note + * twice); a secret's noun already carries it. + */ function nestedProse(envName: string, key: "variables" | "secrets", noun: string) { return { label: `environments[${envName}].${key}`, noun, - where: "the environment", + where: key === "variables" ? `environment "${envName}"` : "the environment", suffix: ` in environment "${envName}"`, }; } From 8c8e3d83bdde02aed6c542450440d657f07d6bbd Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Sun, 13 Sep 2026 07:25:49 -0400 Subject: [PATCH 3/4] fix(contract): a denied request names its operation in parentheses, not as a second verb --- src/sections/contract/errors.ts | 9 ++++++--- test/engine/orchestrate.test.ts | 2 +- test/sections/contract.test.ts | 10 +++++----- test/sections/graphql-contract.test.ts | 2 +- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/sections/contract/errors.ts b/src/sections/contract/errors.ts index 325cff73..6634ada7 100644 --- a/src/sections/contract/errors.ts +++ b/src/sections/contract/errors.ts @@ -60,8 +60,11 @@ export function throwFor( ): never { // The operation label says WHAT was being done in settings-file terms; the raw method/path keeps the request identifiable. // creating ruleset "x" failed - POST /repos/...: 422 ... - const operation = context?.operation ? `${context.operation} failed - ` : ""; - const cause = `${operation}${method} ${path}: ${error.status} ${error.message}`; + // the token was denied POST /repos/... (creating ruleset "x"): 403 ... + const request = `${method} ${path}`; + const outcome = `${error.status} ${error.message}`; + const cause = `${context?.operation ? `${context.operation} failed - ` : ""}${request}: ${outcome}`; + const denied = `${request}${context?.operation ? ` (${context.operation})` : ""}: ${outcome}`; if (isRateLimitError(error)) { // Secondary rate limits arrive as 403 and must not read as missing permissions. throw new Error( @@ -85,7 +88,7 @@ export function throwFor( : sectionGrant(section); throw new PermissionDenied( section.key, - `the token was denied ${cause}${alsoMissing}. To fix, ${grant}${denialHint}`, + `the token was denied ${denied}${alsoMissing}. To fix, ${grant}${denialHint}`, error.status, ); } diff --git a/test/engine/orchestrate.test.ts b/test/engine/orchestrate.test.ts index e6500b73..45c3969c 100644 --- a/test/engine/orchestrate.test.ts +++ b/test/engine/orchestrate.test.ts @@ -72,7 +72,7 @@ describe("runForRepo", () => { 'branches: replacing protection for branch "main" failed - PUT /repos/o/r/branches/main/protection: 404 Branch not found. ' + "The declared branch does not exist on the repo, so its protection cannot be applied; create the branch, or remove it from the settings file"; const deniedPut = - 'the token was denied replacing protection for branch "main" failed - PUT /repos/o/r/branches/main/protection: 404 Not Found ' + + 'the token was denied PUT /repos/o/r/branches/main/protection (replacing protection for branch "main"): 404 Not Found ' + '(a 404 here can also mean the resource does not exist). To fix, grant "Administration" (read and write) under the PAT\'s Repository permissions'; test.each([ ["warn", "Branch not found", "failed", ["branches", "failed"], `error: ${missingBranch}`], diff --git a/test/sections/contract.test.ts b/test/sections/contract.test.ts index 4a544c13..cc14ac0a 100644 --- a/test/sections/contract.test.ts +++ b/test/sections/contract.test.ts @@ -431,7 +431,7 @@ describe("throwFor context enrichment", () => { } expect(thrown).toBeInstanceOf(PermissionDenied); expect((thrown as PermissionDenied).detail).toBe( - 'the token was denied creating ruleset "quality" failed - POST /repos/o/r/rulesets: 403 Resource not accessible. To fix, grant "Administration" (read and write) under the PAT\'s Repository permissions', + 'the token was denied POST /repos/o/r/rulesets (creating ruleset "quality"): 403 Resource not accessible. To fix, grant "Administration" (read and write) under the PAT\'s Repository permissions', ); }); @@ -449,7 +449,7 @@ describe("throwFor context enrichment", () => { { status: 404, message: "Not Found" }, { denied: - 'the token was denied protecting "x" failed - PUT /repos/o/r/branches/x/protection: 404 Not Found (a 404 here can also mean the resource does not exist). To fix, grant "Administration" (read and write) under the PAT\'s Repository permissions', + 'the token was denied PUT /repos/o/r/branches/x/protection (protecting "x"): 404 Not Found (a 404 here can also mean the resource does not exist). To fix, grant "Administration" (read and write) under the PAT\'s Repository permissions', }, ], [ @@ -457,7 +457,7 @@ describe("throwFor context enrichment", () => { { status: 404, message: "Branch not found on fork" }, { denied: - 'the token was denied protecting "x" failed - PUT /repos/o/r/branches/x/protection: 404 Branch not found on fork ' + + 'the token was denied PUT /repos/o/r/branches/x/protection (protecting "x"): 404 Branch not found on fork ' + '(a 404 here can also mean the resource does not exist). To fix, grant "Administration" (read and write) under the PAT\'s Repository permissions', }, ], @@ -466,7 +466,7 @@ describe("throwFor context enrichment", () => { { status: 403, message: "Branch not found" }, { denied: - 'the token was denied protecting "x" failed - PUT /repos/o/r/branches/x/protection: 403 Branch not found. To fix, grant "Administration" (read and write) under the PAT\'s Repository permissions', + 'the token was denied PUT /repos/o/r/branches/x/protection (protecting "x"): 403 Branch not found. To fix, grant "Administration" (read and write) under the PAT\'s Repository permissions', }, ], ] as const)( @@ -1222,7 +1222,7 @@ describe("a marked request's failure is rebuilt on the engine's side of the clie wire: "REST, the client answers a plain 403", run: () => rest(answering(403), true), thrown: - `actions: the token was denied arming the setup failed - PATCH /repos/o/r/code-quality/setup: 403 ${SECRET_RESPONSE_WITHHELD}. ` + + `actions: the token was denied PATCH /repos/o/r/code-quality/setup (arming the setup): 403 ${SECRET_RESPONSE_WITHHELD}. ` + `To fix, grant "Administration" (read and write) under the PAT's Repository permissions; the "oidc_customization_sub" key alone instead needs "Actions" (read and write)`, }, // The controls: unmarked, the same answers render as the client gave them. diff --git a/test/sections/graphql-contract.test.ts b/test/sections/graphql-contract.test.ts index c5f4d855..d30169a5 100644 --- a/test/sections/graphql-contract.test.ts +++ b/test/sections/graphql-contract.test.ts @@ -77,7 +77,7 @@ describe("callGraphql", () => { } expect(thrown).toBeInstanceOf(PermissionDenied); expect((thrown as PermissionDenied).detail).toBe( - 'the token was denied reading repository toggles failed - GRAPHQL RepoToggles: 403 Resource not accessible. To fix, grant "Administration" (read and write) under the PAT\'s Repository permissions', + 'the token was denied GRAPHQL RepoToggles (reading repository toggles): 403 Resource not accessible. To fix, grant "Administration" (read and write) under the PAT\'s Repository permissions', ); }); From 2e71b663c14b8ae6037c2e31d764deffa46154d0 Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Sun, 13 Sep 2026 07:25:49 -0400 Subject: [PATCH 4/4] docs(scenarios): the secret check-drift descriptions say the sealing key is read at apply --- .../actions_secrets/scenarios/actions-secrets-check-drift.yml | 4 ++-- .../agents_secrets/scenarios/agents-secrets-check-drift.yml | 4 ++-- .../scenarios/codespaces-secrets-check-drift.yml | 4 ++-- .../scenarios/dependabot-secrets-check-drift.yml | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/sections/actions_secrets/scenarios/actions-secrets-check-drift.yml b/src/sections/actions_secrets/scenarios/actions-secrets-check-drift.yml index 0d2fd53a..dbaf02eb 100644 --- a/src/sections/actions_secrets/scenarios/actions-secrets-check-drift.yml +++ b/src/sections/actions_secrets/scenarios/actions-secrets-check-drift.yml @@ -3,8 +3,8 @@ description: > Check mode reconciles secrets by existence alone: a declared-but-missing secret is drift, the declared values earn one cannot-verify note, an undeclared live secret stays a keep-note under the default policy, and no - reference is resolved (this scenario deliberately defines no env), so only the list and the sealing key are read - the - key is a read the plan closes over for its sealed writes, in every mode. + reference is resolved (this scenario deliberately defines no env), so only the list is read; the sealing + key is a read the plan closes over for its sealed writes, issued at apply. settings: actions_secrets: - name: DEPLOY_TOKEN diff --git a/src/sections/agents_secrets/scenarios/agents-secrets-check-drift.yml b/src/sections/agents_secrets/scenarios/agents-secrets-check-drift.yml index 09d5876a..90dc6097 100644 --- a/src/sections/agents_secrets/scenarios/agents-secrets-check-drift.yml +++ b/src/sections/agents_secrets/scenarios/agents-secrets-check-drift.yml @@ -4,8 +4,8 @@ description: > declared-but-missing secret is drift, the declared values earn one cannot-verify note, an undeclared live secret stays a keep-note under the default policy, and no reference is resolved (this scenario deliberately - defines no env), so only the list and the sealing key are read - the - key is a read the plan closes over for its sealed writes, in every mode. + defines no env), so only the list is read; the sealing + key is a read the plan closes over for its sealed writes, issued at apply. settings: agents_secrets: - name: AGENT_TOKEN diff --git a/src/sections/codespaces_secrets/scenarios/codespaces-secrets-check-drift.yml b/src/sections/codespaces_secrets/scenarios/codespaces-secrets-check-drift.yml index 2504eb62..5c853bbb 100644 --- a/src/sections/codespaces_secrets/scenarios/codespaces-secrets-check-drift.yml +++ b/src/sections/codespaces_secrets/scenarios/codespaces-secrets-check-drift.yml @@ -4,8 +4,8 @@ description: > declared-but-missing secret is drift, the declared values earn one cannot-verify note, an undeclared live secret stays a keep-note under the default policy, and no reference is resolved (this scenario deliberately - defines no env), so only the list and the sealing key are read - the - key is a read the plan closes over for its sealed writes, in every mode. + defines no env), so only the list is read; the sealing + key is a read the plan closes over for its sealed writes, issued at apply. settings: codespaces_secrets: - name: DOTFILES_PAT diff --git a/src/sections/dependabot_secrets/scenarios/dependabot-secrets-check-drift.yml b/src/sections/dependabot_secrets/scenarios/dependabot-secrets-check-drift.yml index 2e0b7570..0e60849d 100644 --- a/src/sections/dependabot_secrets/scenarios/dependabot-secrets-check-drift.yml +++ b/src/sections/dependabot_secrets/scenarios/dependabot-secrets-check-drift.yml @@ -4,8 +4,8 @@ description: > declared-but-missing secret is drift, the declared values earn one cannot-verify note, an undeclared live secret stays a keep-note under the default policy, and no reference is resolved (this scenario deliberately - defines no env), so only the list and the sealing key are read - the - key is a read the plan closes over for its sealed writes, in every mode. + defines no env), so only the list is read; the sealing + key is a read the plan closes over for its sealed writes, issued at apply. settings: dependabot_secrets: - name: REGISTRY_TOKEN