M3: runtime approved-surface enforcement (field-name validation de-scoped)

[adnaan]

Milestone M3 — runtime approved-surface enforcement, executed per convention 9 (design expanded at kickoff, then implemented). Re-scoped tinkerdown-only at kickoff, and shipped as one phase — the runtime gate — with field-name validation honestly de-scoped.

The kickoff re-scope: tinkerdown-only, not upstream

The plan filed M3 as upstream (WithActionPolicy + introspection). A direct read of both repos showed both would be dead code: tinkerdown routes actions entirely through its own GenericState.HandleAction → RunSQLAction, never through livetemplate.DispatchWithState, and its state is map-based (nothing to reflect over). Unlike M2 (which consumed the upstream Validate()), an upstream hook here has zero tinkerdown consumption. The upstream WithActionPolicy is deferred as separable framework work.

What shipped — the runtime approved-surface gate (a3513de)

When a project declares a generation: block, an action invoked outside the approved surface is rejected server-side on every path a caller can reach:

• WS-custom action → EnforceApprovedAction (name only)
• WS-builtin write → EnforceApprovedSource (bound source)
• webhook → EnforceApprovedAction → 403, not executed

This is the server-side gate confirm: never was (a dialog can't be server-verified) — closing the M1 Risk 758 defense-in-depth hole. Opt-in: no generation: block, no gate.

The key subtlety: the approved surface is two surfaces. A custom action gates on ApprovedAction only, not its source — because PII's approved actions deliberately target access_store, a writable store kept out of generation.sources (unbindable by any generated app). A builtin gates on ApprovedSource(boundSource). Conflating them would have rejected the PII demo.

Field-name validation — de-scoped (871a9c0), not silently dropped

The Audit found it can't be built cleanly: it introspects a source's schema via PRAGMA on the live DB, but tinkerdown's sqlite *.db is gitignored and built at serve — so at validate time (CI, generate→validate→serve) no DB exists, the check skips the entire corpus, yet would fire unvetted on a dev machine's stray .db. That's the self-certifying-guard landmine this project kept killing (never-fires-in-CI + fires-unvetted-on-dev = negative expected value). The M2 feed-forward it was meant to close is re-filed to § Risks with the real prerequisite named: a validate-time schema source (parse CREATE TABLE from a seed, or a declared columns: schema) — a distinct introspection capability, backlog/M4. The plan's now-overclaiming M3 statements are walked back to match.

Verification

Every gate path has a reject test (config predicates; WS custom + builtin rejected before execution; webhook 403-and-not-executed; opt-in when no generation block), and TestPIIAccessApproval passes with approved actions running through the gate. Full suite green. Simplifier: diff already clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h

[claude]

Review: M3 runtime approved-surface enforcement

Reviewed the diff (internal/config/config.go, internal/runtime/state.go, internal/server/webhook.go, internal/server/websocket.go, plus the three new test files). Overall this is a clean, well-targeted change: EnforceApprovedAction/EnforceApprovedSource are nil-safe (opt-in when no generation: block), the two predicates are correctly kept separate (action-name-only vs. bound-source, matching the access_store/PII rationale explained in the PR body), and the gate is wired into all three call sites the description claims (WS-custom, WS-builtin write, webhook-direct). SetPageConfig being made unconditional in websocket.go is the right fix — otherwise builtin-only pages (no custom actions) would never get cfg and the gate would silently no-op for them.

Correctness gap: the "run" builtin action (exec sources) is not gated

internal/runtime/state.go's builtinActions map explicitly includes "run": true, and its doc comment states these are "governed by whether the source itself is approved and writable." But in HandleAction's switch, "run" (internal/runtime/state.go:298) dispatches straight to s.runExec(data) with no EnforceApprovedSource (or any) check — unlike "add"/"toggle"/"delete"/"update" at state.go:307-313, which now correctly call s.cfg.EnforceApprovedSource(s.sourceName) before executing.

This means a page whose frontmatter binds an lvt-source block to an exec source that is not in generation.sources — the exact "document bound one anyway" scenario EnforceApprovedSource's own doc comment calls out as the defense-in-depth target — can still have its exec command run via a crafted "run" WS message, with caller-supplied args (data[s.Args[i].Name] in runExec, internal/runtime/actions.go:38+), completely outside the approved surface. --allow-exec still gates whether exec sources can exist at all, but that's an all-or-nothing operator flag, not the per-source approval this milestone is meant to add.

Given exec is the highest-risk builtin (arbitrary command execution vs. a scoped SQL write), and the CHANGELOG/PR body both claim "every path a caller can reach" is now enforced, this looks like an unintentional gap rather than a deliberate de-scope — worth either adding the EnforceApprovedSource check to the "run" case or explicitly documenting it as excluded (and why) if that's intentional.

Everything else

• EnforceApprovedAction/EnforceApprovedSource in config.go correctly mirror ApprovedAction/ApprovedSource's nil-config/nil-Generation handling, so a nil *Config receiver stays safe through the new WS/webhook call sites.
• Webhook gate placement (webhook.go:279-286) is sound — it runs after auth/signature validation and action-existence lookup but strictly before actionHandler is invoked, and the added test (webhook_enforce_test.go) confirms the unapproved case never reaches mock.calls.
• Test coverage for the shipped surface (config predicates, WS custom + builtin, webhook 403-and-not-executed, opt-in-when-no-generation-block) is solid and each test asserts on the specific rejection path rather than just "an error occurred."
• Minor: EnforceApprovedAction/EnforceApprovedSource re-implement the c == nil || c.Generation == nil guard that ApprovedAction/ApprovedSource already have, rather than composing (can't purely delegate since the "no gate" vs. "not approved" distinction requires the early return, but flagging in case there's a cleaner shared helper). Not worth blocking on.

No issues found with the de-scoping of field-name validation (Phase 2) — the reasoning in the PR body (schema only available on the gitignored, seed-built runtime DB, making the check vacuous in CI and unvetted on dev) is sound and appropriately re-filed to Risks rather than silently dropped.