From b245875a05398687e131fb7b17952c67de8a45fc Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Mon, 5 Oct 2026 04:07:05 -0400 Subject: [PATCH] refactor(sections): give secretValuesOf a typed signature and drop the container checks it made necessary secretValuesOf took `unknown` and re-proved the container and each entry with typeof and Array.isArray checks, because the erased list-section path once fed it an erased value. Every producer now hands it zod's output: validateSectionShapes calls the hook only after the shape parse succeeded, and collectSecretReferences takes the validated document. The function takes the declared list form listEntries() already models, so the extract callback receives the section's own entry type. The callers pass the typed value and lose their twins of the shape parse: listSecretValues no longer checks that name and value are strings, the environments hook no longer casts the entry or guards its name, and secretValuesFor passes the erased declared value instead of unknown while keeping the narrowings valueAt()'s erased walk needs. gated() in plan.ts constrains its parameter to an object whose members are all functions and drops the typeof branch both call sites could never take. The two tests that fed secretValues malformed containers through double casts had no producer left to pin: the webhooks one is deleted and the environments one keeps only the typed inputs. A caller that violates the type with null or a scalar now gets a TypeError from listEntries instead of an empty list; the type is the fix. --- src/sections/contract/module.ts | 34 +++++-------------- src/sections/contract/plan.ts | 11 +++--- src/sections/environments/index.ts | 26 ++++++-------- src/sections/shared/list-section.ts | 15 ++++++-- src/sections/shared/secrets-engine.ts | 17 ++++------ .../environments/environments.test.ts | 11 ++---- test/sections/webhooks/webhooks.test.ts | 8 ----- 7 files changed, 45 insertions(+), 77 deletions(-) diff --git a/src/sections/contract/module.ts b/src/sections/contract/module.ts index f0dd68923..87c45624a 100644 --- a/src/sections/contract/module.ts +++ b/src/sections/contract/module.ts @@ -756,33 +756,15 @@ export interface DeclaredSecretValue { } /** - * The secret values of a list section's declared value, one `extract` per entry. The engine hands it zod's output - * (engine/validate.ts after the shape parse, engine/secrets.ts the validated document), but the erased list - * declaration and a library caller of sectionModule() reach it as `unknown`, so a malformed container or entry - * contributes nothing rather than throwing, and the actionable error always comes from validation. - */ -export function secretValuesOf( - declared: unknown, - extract: (entry: Readonly>) => readonly DeclaredSecretValue[], + * The secret values of a list section's declared value in either form, one `extract` per entry. Every caller hands + * it zod's output (engine/validate.ts after the shape parse, engine/secrets.ts the validated document), so the + * entries are the section's own type and a malformed value is validation's to report, never this walk's. + */ +export function secretValuesOf( + declared: readonly E[] | { readonly entries: readonly E[] }, + extract: (entry: E) => readonly DeclaredSecretValue[], ): DeclaredSecretValue[] { - const isWrapper = - typeof declared === "object" && - declared !== null && - !Array.isArray(declared) && - Array.isArray((declared as { entries?: unknown }).entries); - if (!Array.isArray(declared) && !isWrapper) { - return []; - } - // "keep" is a placeholder: only the entries are read. - const { entries } = undeclaredPolicy( - declared as readonly unknown[] | UndeclaredPolicyList, - "keep", - ); - return entries.flatMap((entry) => - typeof entry === "object" && entry !== null && !Array.isArray(entry) - ? [...extract(entry as Readonly>)] - : [], - ); + return listEntries(declared).flatMap((entry) => [...extract(entry)]); } /** diff --git a/src/sections/contract/plan.ts b/src/sections/contract/plan.ts index 6207cb197..909f3500c 100644 --- a/src/sections/contract/plan.ts +++ b/src/sections/contract/plan.ts @@ -150,14 +150,15 @@ export interface ExecTools { resolveSecret(reference: string): string; } +/** What every member of a bound port is, whatever the helper takes. */ +type Helper = (...args: never[]) => unknown; + /** The runtime shape of the gated ports: the token is discarded, so the gate is the type alone. */ -function gated(bound: T): object { +function gated(bound: T): object { return Object.fromEntries( - Object.entries(bound).map(([name, helper]) => [ + Object.entries(bound).map(([name, helper]) => [ name, - typeof helper === "function" - ? (_exec: ExecTools, ...args: unknown[]) => helper(...args) - : helper, + (_exec: ExecTools, ...args: never[]) => helper(...args), ]), ); } diff --git a/src/sections/environments/index.ts b/src/sections/environments/index.ts index 04affa2d3..6ffafc36c 100644 --- a/src/sections/environments/index.ts +++ b/src/sections/environments/index.ts @@ -12,7 +12,6 @@ import type { SectionFailure } from "../contract/errors.js"; import { liveByIdentity, liveIdentity } from "../contract/live.js"; import { - type DeclaredSecretValue, declaredEntries, identifiedBy, type KeyedListLayering, @@ -137,21 +136,16 @@ export const environmentsSection = { endpoints: ENDPOINTS, graphql: GRAPHQL_OPS, shape: routed(layeredList(EnvironmentsConfig)), - /** - * Labels carry the environment: sibling environments can declare same-named secrets. - * A malformed container contributes nothing rather than throwing, so the actionable error - * always comes from shape validation. - */ - secretValues(declared: unknown): DeclaredSecretValue[] { - return secretValuesOf(declared, (entry) => { - const env = entry as EnvironmentConfig; - const where = - typeof env.name === "string" ? `environment "${env.name}"` : "an unnamed environment"; - return listSecretValues(env.secrets).map(({ label, value }) => ({ - label: `${label} of ${where}`, - value, - })); - }); + /** Labels carry the environment: sibling environments can declare same-named secrets. */ + secretValues(declared) { + return secretValuesOf(declared, (env) => + env.secrets === undefined + ? [] + : listSecretValues(env.secrets).map(({ label, value }) => ({ + label: `${label} of environment "${env.name}"`, + value, + })), + ); }, validate(desired) { const issues = IDENTITY.validate(desired); diff --git a/src/sections/shared/list-section.ts b/src/sections/shared/list-section.ts index 9e589e155..060c7b075 100644 --- a/src/sections/shared/list-section.ts +++ b/src/sections/shared/list-section.ts @@ -27,7 +27,15 @@ import { snapshotList } from "./list-section-snapshot.js"; import { identityClaims, pathOf, valueAt } from "./list-section-write.js"; import { knobbed, routed } from "./schema-helpers.js"; -function secretValuesFor(decl: ErasedDecl, declared: unknown): DeclaredSecretValue[] { +/** + * valueAt() walks the erased entry, so a declared secret field reads as `unknown`: absent on an entry that leaves + * the optional field out, a string otherwise, like the identity beside it. The narrowing is that walk's, not a + * second proof of the shape. + */ +function secretValuesFor( + decl: ErasedDecl, + declared: ErasedDeclared, +): DeclaredSecretValue[] { const fields = decl.secrets ?? []; return secretValuesOf(declared, (entry) => fields.flatMap((field) => { @@ -66,7 +74,10 @@ export function listSection< shape: routed(knobbed(decl.entry)), ...(decl.secrets === undefined ? {} - : { secretValues: (declared: Declared) => secretValuesFor(erased, declared) }), + : { + secretValues: (declared: Declared) => + secretValuesFor(erased, declared as unknown as ErasedDeclared), + }), layering: { keys: (entry) => identityClaims(erased.identity, entry), keyField: decl.identity.field, diff --git a/src/sections/shared/secrets-engine.ts b/src/sections/shared/secrets-engine.ts index 5d736fc64..e2ffdff8e 100644 --- a/src/sections/shared/secrets-engine.ts +++ b/src/sections/shared/secrets-engine.ts @@ -121,17 +121,12 @@ export function secretKey(name: string): string { } /** 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") { - return []; - } - const label = - typeof entry.name === "string" - ? `the secret entry "${entry.name}"` - : "an unnamed secret entry"; - return [{ label, value: entry.value }]; - }); +export function listSecretValues( + declared: SecretEntry[] | UndeclaredPolicyList, +): DeclaredSecretValue[] { + return secretValuesOf(declared, (entry) => [ + { label: `the secret entry "${entry.name}"`, value: entry.value }, + ]); } /** diff --git a/test/sections/environments/environments.test.ts b/test/sections/environments/environments.test.ts index 9e70ddb19..50e02eefa 100644 --- a/test/sections/environments/environments.test.ts +++ b/test/sections/environments/environments.test.ts @@ -556,23 +556,16 @@ describe("environments nested secrets validation and shape", () => { ); }); - test("secretValues walks every entry's secrets list and survives malformed containers", () => { - // The double cast feeds secretValues a pre-validation document slice on purpose: its contract is defensiveness against any merged value. + test("secretValues walks every entry's secrets list in either nested form and labels each by its environment", () => { const values = environmentsSection.secretValues?.([ { name: "a", secrets: [{ name: "X", value: "$X" }] }, { name: "b", secrets: { entries: [{ name: "Y", value: "$Y" }] } }, { name: "c" }, - { name: "d", secrets: "garbage" }, - "not-an-entry", - ] as unknown as EnvironmentConfig[]); + ]); expect(values).toEqual([ { label: 'the secret entry "X" of environment "a"', value: "$X" }, { label: 'the secret entry "Y" of environment "b"', value: "$Y" }, ]); - // A non-list section value contributes nothing (validation reports it). - expect( - environmentsSection.secretValues?.({ not: "a list" } as unknown as EnvironmentConfig[]), - ).toEqual([]); }); }); diff --git a/test/sections/webhooks/webhooks.test.ts b/test/sections/webhooks/webhooks.test.ts index 61a58df98..5dc151081 100644 --- a/test/sections/webhooks/webhooks.test.ts +++ b/test/sections/webhooks/webhooks.test.ts @@ -125,14 +125,6 @@ describe("webhooks secretValues", () => { }), ).toEqual([{ label: 'the webhook "https://a.test" config.secret', value: "$B" }]); }); - - test("malformed containers return [] and leave the error to validation", () => { - // The extractor faces any merged value, so the double cast feeds it a pre-validation value on purpose; validation is where the user gets the - // message. - for (const malformed of [null, "hooks", 42, { _undeclared: "keep" }, [null, "x"]]) { - expect(webhooksSection.secretValues?.(malformed as unknown as WebhookConfig[])).toEqual([]); - } - }); }); describe("webhooks plan", () => {