diff --git a/docs/reference/contracts/workflow-yaml-spec.md b/docs/reference/contracts/workflow-yaml-spec.md index 244046b8..5331289f 100644 --- a/docs/reference/contracts/workflow-yaml-spec.md +++ b/docs/reference/contracts/workflow-yaml-spec.md @@ -88,6 +88,10 @@ inputs: default: 'team@example.com' ``` +An input `name` must be a **referenceable identifier** — `[A-Za-z0-9_-]+` (letters, digits, `_` or `-`), +the same charset the `{{inputs.}}` head accepts — so a name like `my name` or `a.b` that could never +be referenced is rejected at parse (ADR-0023). + `secret`-typed inputs are resolved through the secret store, never written into run logs or the workflow file. They are also **masked in event payloads**: a `secret` input's value is redacted from the `run:started.inputs` payload (and any other event that echoes inputs), so a secret never reaches a surface, an IPC channel, or a persisted run log — see the masking rule in [sse-event-schema.md](sse-event-schema.md). See also [../desktop/keychain-and-secrets.md](../desktop/keychain-and-secrets.md). An input may carry an optional **`validation`** object the engine checks before a run starts; a violating input fails fast and the run never begins: @@ -117,7 +121,7 @@ inputs: ## Context and interpolation -`context` declares named values available throughout the workflow as `{{ctx.key}}`. Interpolation uses `{{ ... }}` syntax everywhere (inputs, context, prompt templates, message templates, edge/condition expressions). +`context` declares named values available throughout the workflow as `{{ctx.key}}`. Interpolation uses `{{ ... }}` syntax everywhere (inputs, context, prompt templates, message templates, edge/condition expressions). A context `key`, like an input `name`, must be a referenceable identifier (`[A-Za-z0-9_-]+`). ```yaml context: diff --git a/docs/roadmap/current.md b/docs/roadmap/current.md index cb5938ea..9cf554ca 100644 --- a/docs/roadmap/current.md +++ b/docs/roadmap/current.md @@ -2,14 +2,14 @@ > Status: Living -> Last updated: 2026-06-11 +> Last updated: 2026-06-12 - **Related**: [README.md](README.md), [phases/phase-0-foundations.md](phases/phase-0-foundations.md), [phases/phase-1-engine-and-llm.md](phases/phase-1-engine-and-llm.md), [../project-structure.md](../project-structure.md), [../tech-stack.md](../tech-stack.md) This page tracks what is active **right now** and the immediate next concrete actions. The full phase plan and the global milestone spine are in [README.md](README.md); the granular work breakdown for the active phase is in -[phases/phase-0-foundations.md](phases/phase-0-foundations.md). +[phases/phase-1-engine-and-llm.md](phases/phase-1-engine-and-llm.md). ## Where we are @@ -145,8 +145,10 @@ now scaffolded with a pure-TypeScript `WorkflowYAMLParser` that parses and valid > off the M3 critical path); the **`turn_limit` `ErrorCode`** (a hard session turn cap, distinct from > the `[chat].max_messages` trim threshold); the **reserved `on_error` edge kind** > (workflow-yaml-spec.md, not authorable in v1.0); and a CI **engine dependency-allowlist guard** + the -> pnpm install-script allowlist. No Phase-1 work changed; **1.K has since landed (PR #13)** and **1.L is -> the next workstream**. +> pnpm install-script allowlist. No Phase-1 work changed; **1.K has since landed (PR #13)**, and +> **1.L has since landed (PR #14, 2026-06-12)**; **1.L2 (the `{{ … }}` interpolation engine + the +> parse-time secret-taint gate) is in review (PR #15)** — once it merges, **1.M (DAG builder + +> `RunPlan`) is the next workstream**. Carry-over hardening is tracked in [deferred-tasks.md](deferred-tasks.md) — pick items up as Phase 1 first touches each file. diff --git a/docs/roadmap/phases/phase-1-engine-and-llm.md b/docs/roadmap/phases/phase-1-engine-and-llm.md index c6fb63c0..7ea553a5 100644 --- a/docs/roadmap/phases/phase-1-engine-and-llm.md +++ b/docs/roadmap/phases/phase-1-engine-and-llm.md @@ -470,7 +470,7 @@ schemas; the run-event count-test is green at the new total; `tsc` + the seam fe today is the count-pinned unit test, **not** the DB-migration drift gate — `run_events.event_type` is unconstrained text — so the test total must be updated deliberately.) -### 1.L — `WorkflowYAMLParser` (parse + validate) — *critical path* +### 1.L — `WorkflowYAMLParser` (parse + validate) — *critical path* · ✅ **Done (PR #14, 2026-06-12)** The engine entry point: load a `.relavium.yaml` and validate it against the `@relavium/shared` `WorkflowSchema` (post-1.L.0) before any LLM call. @@ -504,16 +504,19 @@ of every node, so it is sequenced before 1.M/1.O/1.P.) It is distinct from the J (1.AB): `{{ … }}` is string templating; `condition`/`transform`/`merge_fn` are JS evaluated in the sandbox. **Tasks:** -- Evaluate `{{ … }}` against the run scope — `inputs`, `ctx`, `run.outputs` (keyed by node id), `secrets` — - with the pipe-filter registry (`| read_file`, `| json`, `| length`, `| default`, …) per - [workflow-yaml-spec.md](../../reference/contracts/workflow-yaml-spec.md). +- Evaluate `{{ … }}` against the run scope — the three authored namespaces `inputs`, `ctx`, and + `run.outputs` (keyed by node id); a `secrets.*` reference is recognized by the lexer only so the + resolver/taint gate can reject it — with the pipe-filter registry (`| read_file`, `| json`, + `| length`, `| default("…")` — the argument-taking fallback applied when the value is missing, …) + per [workflow-yaml-spec.md](../../reference/contracts/workflow-yaml-spec.md). - **Eager-once, immutable cached context:** a node's inputs are resolved once into a frozen snapshot, so a re-run/replay is deterministic (aligned with the checkpoint + idempotency model, 1.R). - Enforce the **transitive parse-time secret taint** [ADR-0029(c)](../../decisions/0029-tool-policy-hardening.md) mandates "by the parser": a `secret`-typed value (or anything derived from one) is rejected from - `prompt_template` / tool text, allowed only in credential/header fields. Raise a typed `InterpolationError` - (key + workspace-relative location + node id; no absolute paths, no secret values) per - [error-handling.md](../../standards/error-handling.md). + `prompt_template` / tool text, allowed only in credential/header fields. Raise a typed, + field-named parse error (`WorkflowSecretLeakError`) — names only, no absolute paths, no secret + values — per [error-handling.md](../../standards/error-handling.md). (A *runtime* resolver failure + is the separate `InterpolationError`.) **Acceptance:** interpolation resolves refs + filters correctly; a secret routed into prompt/tool text is rejected at parse with a field-named, secret-free error; re-resolving a node yields an identical frozen scope. @@ -608,10 +611,20 @@ Executes a single agent node end-to-end against `@relavium/llm`. - Tool results enter message assembly **as data through the typed untrusted boundary** ([security-review.md §Prompt-injection posture](../../standards/security-review.md#prompt-injection-posture), binding): never into `system`, never string-concatenated into an instruction template. + **This also covers resolved-interpolation content**: 1.L2's `resolveTemplate` returns a flat + string, dropping provenance, so a field that drew on `read_file` / `run.outputs` (untrusted) must + be carried as untrusted into a `user`/`tool` position by this layer, never `system`. +- **Re-apply secret taint when populating `run.outputs`** (ADR-0029(c) follow-up): the 1.L2 + parse-time gate covers only the authored `{{ … }}` template graph. Any node output derived from a + `secret` — e.g. a `secret`-typed input surfaced *verbatim* via an `input` node's output — **must be + marked tainted when it is placed into `run.outputs`**, so a later `{{run.outputs["…"]}}` reference + into agent/human text is rejected the same way an authored secret reference is. The parse-time gate + remains responsible only for the authored template graph. **Acceptance:** an agent node with a tool streams tokens, performs a tool round-trip, emits a correct `cost:updated`, and completes with `node:completed`; a forced -provider error drives the fallback chain before the node is considered failed. +provider error drives the fallback chain before the node is considered failed; a `secret`-typed +input cannot reach agent/human text through a node output (taint re-applied at `run.outputs`). ### 1.P — Node-type handlers (condition / fan-out / fan-in / transform / input / output) diff --git a/docs/standards/security-review.md b/docs/standards/security-review.md index b96371d5..5d344d5b 100644 --- a/docs/standards/security-review.md +++ b/docs/standards/security-review.md @@ -291,8 +291,10 @@ redaction rules live in [logging-and-observability.md](logging-and-observability Any change to: key handling or the keychain bridge, IPC commands, the desktop Rust-delegated egress path (`llm_stream` / `Channel`), provider base-URL handling, the `http_request` tool or MCP server-URL handling (the other two SSRF egress -paths), the `run_command` sandbox, node `tools:` narrowing or `secret`-typed input -handling, prompt/tool-call construction, **media byte delivery (`read_media` / Range / upload) and the -media `url` carrier**, the DB encryption path, or a new dependency. For +paths), the `run_command` sandbox, **the host file reader behind the `read_file` interpolation +filter (`ResolverCapabilities.readFile`) — which must jail to the workspace root and reject path +traversal, a duty the pure engine delegates to each host**, node `tools:` narrowing or +`secret`-typed input handling, prompt/tool-call construction, **media byte delivery (`read_media` / +Range / upload) and the media `url` carrier**, the DB encryption path, or a new dependency. For **managed mode**, also: the gateway authn/z path, key-pool selection, the metering/billing path, and the master-key vault. When in doubt, run the checklist. diff --git a/packages/core/src/errors.test.ts b/packages/core/src/errors.test.ts index a936c8d5..8acea289 100644 --- a/packages/core/src/errors.test.ts +++ b/packages/core/src/errors.test.ts @@ -1,6 +1,13 @@ import { describe, expect, it } from 'vitest'; -import { WorkflowSyntaxError, WorkflowValidationError, type WorkflowIssue } from './errors.js'; +import { + InterpolationError, + WorkflowSecretLeakError, + WorkflowSyntaxError, + WorkflowValidationError, + type SecretLeak, + type WorkflowIssue, +} from './errors.js'; const issue = (field: string): WorkflowIssue => ({ field, message: 'bad' }); @@ -46,3 +53,60 @@ describe('WorkflowSyntaxError', () => { expect(err.column).toBeUndefined(); }); }); + +describe('WorkflowSecretLeakError', () => { + const leak = (over: Partial = {}): SecretLeak => ({ + location: 'node `n`.prompt_template', + secret: 'inputs.api_key', + ...over, + }); + + it('summarizes the first leak, names the field/symbol, and cites the ADR', () => { + const err = new WorkflowSecretLeakError([leak()]); + expect(err.code).toBe('secret_interpolation'); + expect(err.message).toBe( + 'node `n`.prompt_template interpolates the secret `inputs.api_key` — secrets are rejected from agent/human text (ADR-0029)', + ); + }); + + it('includes a `via` hop and a plural "more" suffix', () => { + const err = new WorkflowSecretLeakError([ + leak({ secret: 'ctx.creds', via: 'inputs.api_key' }), + leak(), + leak(), + ]); + expect(err.message).toContain('the secret `ctx.creds` (via `inputs.api_key`)'); + expect(err.message).toContain('(and 2 more leaks)'); + }); + + it('uses a singular "more leak" suffix for exactly two leaks', () => { + expect(new WorkflowSecretLeakError([leak(), leak()]).message).toContain('(and 1 more leak)'); + }); + + it('summarizes an empty leak list defensively', () => { + expect(new WorkflowSecretLeakError([]).message).toBe('secret interpolation rejected'); + }); + + it('omits a `via` that equals the secret (a direct, un-laundered reference)', () => { + const err = new WorkflowSecretLeakError([leak({ via: 'inputs.api_key' })]); + expect(err.message).not.toContain('(via'); + }); +}); + +describe('InterpolationError', () => { + it('carries a typed code and the offending reference as location', () => { + const err = new InterpolationError('unknown_filter', 'unknown filter `nope`', { + location: '{{inputs.x | nope}}', + }); + expect(err.code).toBe('unknown_filter'); + expect(err.location).toBe('{{inputs.x | nope}}'); + expect(err.cause).toBeUndefined(); + }); + + it('keeps a host error on cause and omits location when not given', () => { + const cause = new Error('inner'); + const err = new InterpolationError('read_file_failed', 'read failed', { cause }); + expect(err.cause).toBe(cause); + expect(err.location).toBeUndefined(); + }); +}); diff --git a/packages/core/src/errors.ts b/packages/core/src/errors.ts index ce6f58b1..eea4959e 100644 --- a/packages/core/src/errors.ts +++ b/packages/core/src/errors.ts @@ -1,15 +1,20 @@ /** - * Typed, discriminated errors thrown by the engine's `WorkflowYAMLParser` (1.L). They mirror the - * `@relavium/llm` `LlmConfigError` pattern — a base class with a stable `code` discriminant and - * structured, secret-free context, narrowed on `code` and never on `message` - * (docs/standards/error-handling.md). The user-facing fields — `message`, `issues`, `field`, the - * `source` label, and the line/column — name the offending field/node and never carry an authored - * value, a stack trace, or an absolute path. An internal `cause`, where attached, is a non-secret - * diagnostic (a YAML rule, never the source text) kept for logs per error-handling.md; the raw - * ZodError is deliberately NOT attached, as it can carry an authored `received` value. + * Typed, discriminated errors thrown by the engine's `WorkflowYAMLParser` (1.L) and the `{{ … }}` + * interpolation engine (1.L2). They mirror the `@relavium/llm` `LlmConfigError` pattern — a base + * class with a stable `code` discriminant and structured, secret-free context, narrowed on `code` + * and never on `message` (docs/standards/error-handling.md). The user-facing fields — `message`, + * `issues`, `field`, `leaks`, the `source` label, and the line/column — name the offending + * field/node/symbol and never carry an authored value, a stack trace, or an absolute path. An + * internal `cause`, where attached, is a non-secret diagnostic (a YAML rule, or a host `readFile` + * error) kept for logs per error-handling.md; the raw ZodError is deliberately NOT attached, as it + * can carry an authored `received` value. + * + * Two families: the parse-time {@link WorkflowParseError} (syntax / schema / secret-leak), thrown by + * `parseWorkflow`; and the runtime {@link InterpolationError}, thrown while resolving a template + * against a run scope. */ -export type WorkflowParseErrorCode = 'invalid_yaml' | 'schema_validation'; +export type WorkflowParseErrorCode = 'invalid_yaml' | 'schema_validation' | 'secret_interpolation'; /** One field-named validation problem — the unit the VS Code language server later renders. */ export interface WorkflowIssue { @@ -19,6 +24,23 @@ export interface WorkflowIssue { readonly message: string; } +/** + * One rejected secret interpolation (ADR-0029(c)). Every field is a *name* — an authored input name, + * context key, or field locator — never a resolved value, so the finding is safe to surface and log. + */ +export interface SecretLeak { + /** Where the secret was interpolated — e.g. ``node `scan`.prompt_template``. */ + readonly location: string; + /** The tainted symbol referenced at that site — e.g. `inputs.api_key` or `ctx.creds` (a name). */ + readonly secret: string; + /** + * The **immediate** deeper tainted symbol, when laundered through a `context` entry or `input` + * default — e.g. `inputs.api_key`. This is a single hop (the direct predecessor), not the full + * chain; v1.0 surfaces one hop for a concise message. + */ + readonly via?: string; +} + /** Base for every parser error — callers narrow on `code`, never on `message`. */ export abstract class WorkflowParseError extends Error { abstract readonly code: WorkflowParseErrorCode; @@ -78,3 +100,71 @@ function summarize(issues: readonly WorkflowIssue[]): string { const more = rest > 0 ? ` (and ${rest} more issue${suffix})` : ''; return `${first.field}: ${first.message}${more}`; } + +/** + * A `secret`-typed value — or anything transitively derived from one through a `context` entry or an + * `input` default — reaches agent/human text (`prompt_template`, `system_prompt[_append]`, + * `message_template`, `assignee`). Rejected at parse so a run never starts on it (ADR-0029(c)). The + * message names the offending field and the tainted symbol; it never carries the secret's value. + */ +export class WorkflowSecretLeakError extends WorkflowParseError { + readonly code = 'secret_interpolation'; + readonly leaks: readonly SecretLeak[]; + + constructor(leaks: readonly SecretLeak[], opts?: { source?: string; cause?: unknown }) { + super(summarizeLeaks(leaks), opts?.source, opts?.cause); + this.name = 'WorkflowSecretLeakError'; + this.leaks = leaks; + } +} + +function summarizeLeaks(leaks: readonly SecretLeak[]): string { + const first = leaks[0]; + if (first === undefined) { + return 'secret interpolation rejected'; + } + const via = + first.via !== undefined && first.via !== first.secret ? ` (via \`${first.via}\`)` : ''; + const rest = leaks.length - 1; + const suffix = rest === 1 ? '' : 's'; + const more = rest > 0 ? ` (and ${rest} more leak${suffix})` : ''; + return `${first.location} interpolates the secret \`${first.secret}\`${via} — secrets are rejected from agent/human text (ADR-0029)${more}`; +} + +/** Stable discriminant for a runtime interpolation failure — callers narrow on `code`, not `message`. */ +export type InterpolationErrorCode = + | 'unresolved_reference' // a `{{ … }}` head/path resolved to no value (and no `| default(…)` rescued it) + | 'unknown_namespace' // a reference reads from a namespace the resolver does not serve (e.g. `secrets`) + | 'unknown_filter' // a pipe filter name is not in the registry + | 'filter_arity' // a filter was given the wrong number of arguments + | 'filter_type' // a filter cannot apply to the value's type (e.g. `length` on a number) + | 'unserializable' // a reference resolved to an object/array used as text without a `| json` filter + | 'invalid_path' // a malformed property/index access after the head + | 'read_file_unavailable' // the `read_file` filter ran without a host `readFile` capability + | 'read_file_failed' // the host `readFile` capability threw (cause kept for logs, off the message) + | 'aborted'; // the run's `AbortSignal` fired mid-resolution (cooperative cancellation) + +/** + * A runtime interpolation failure raised while *resolving* `{{ … }}` against a run scope (1.L2) — + * distinct from the parse-time {@link WorkflowParseError} family. User-facing and secret-free: the + * message names the offending reference (its verbatim `{{ … }}`) and never a resolved value; an + * absolute path from a host `readFile` failure stays on the `cause` (for logs), never in the message. + */ +export class InterpolationError extends Error { + readonly code: InterpolationErrorCode; + /** The offending `{{ … }}` occurrence, verbatim — names the reference, never a resolved value. */ + readonly location?: string; + + constructor( + code: InterpolationErrorCode, + message: string, + opts?: { location?: string; cause?: unknown }, + ) { + super(message, opts?.cause === undefined ? undefined : { cause: opts.cause }); + this.name = 'InterpolationError'; + this.code = code; + if (opts?.location !== undefined) { + this.location = opts.location; + } + } +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index b682b3b9..6cc20d98 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -6,13 +6,24 @@ * Code extension host, and Bun alike. */ -// WorkflowYAMLParser (1.L) — parse + validate a `.relavium.yaml` string into a typed definition. +// WorkflowYAMLParser (1.L / 1.L2) — parse + validate + static interpolation gates into a typed def. export { parseWorkflow } from './parser.js'; export type { WorkflowDefinition, ParseWorkflowOptions } from './parser.js'; // Typed, field-named, secret-free parse/validation errors — narrow on `code`, never on `message`. -export { WorkflowParseError, WorkflowSyntaxError, WorkflowValidationError } from './errors.js'; -export type { WorkflowParseErrorCode, WorkflowIssue } from './errors.js'; +export { + WorkflowParseError, + WorkflowSyntaxError, + WorkflowValidationError, + WorkflowSecretLeakError, + InterpolationError, +} from './errors.js'; +export type { + WorkflowParseErrorCode, + WorkflowIssue, + SecretLeak, + InterpolationErrorCode, +} from './errors.js'; // Structured, un-evaluated interpolation references — the view the DAG builder (1.M) consumes. export { parseTemplate, templateReferences } from './interpolation/references.js'; @@ -24,4 +35,11 @@ export type { FilterArg, } from './interpolation/references.js'; export { collectReferences } from './interpolation/collect.js'; -export type { ReferenceSite } from './interpolation/collect.js'; +export type { ReferenceSite, ReferenceSiteCategory } from './interpolation/collect.js'; + +// Static interpolation analyses (1.L2) — also consumed by the future VS Code language server. +export { analyzeSecretTaint, analyzePreRunReferences } from './interpolation/analyze.js'; + +// The `{{ … }}` runtime resolver (1.L2) — evaluate templates against a run scope, eager-once context. +export { resolveTemplate, resolveContext } from './interpolation/resolve.js'; +export type { RunScope, ResolverCapabilities } from './interpolation/scope.js'; diff --git a/packages/core/src/interpolation/analyze.test.ts b/packages/core/src/interpolation/analyze.test.ts new file mode 100644 index 00000000..a4f9e6cb --- /dev/null +++ b/packages/core/src/interpolation/analyze.test.ts @@ -0,0 +1,401 @@ +import { describe, expect, it } from 'vitest'; + +import { WorkflowSchema } from '@relavium/shared'; + +import { WorkflowSecretLeakError, WorkflowValidationError } from '../errors.js'; +import { parseWorkflow } from '../parser.js'; + +import { analyzePreRunReferences, analyzeSecretTaint } from './analyze.js'; + +/** A schema-valid inline agent the leak fixtures bind their agent nodes to. */ +const AGENT = ` agents: + - id: ag + name: Ag + model: claude-sonnet-4-6 + provider: anthropic + system_prompt: 'system'`; + +/** Parse `yaml`, asserting it throws a WorkflowSecretLeakError; returns it for leak assertions. */ +function expectLeak(yaml: string): WorkflowSecretLeakError { + try { + parseWorkflow(yaml); + } catch (err) { + if (!(err instanceof WorkflowSecretLeakError)) { + throw err; // an unexpected error type — surface it rather than mis-narrowing + } + return err; + } + throw new Error('expected parseWorkflow to reject the secret interpolation'); +} + +describe('analyzeSecretTaint — rejected leaks (via parseWorkflow)', () => { + it('rejects a secret-typed input interpolated directly into a prompt', () => { + const err = expectLeak(`schema_version: '1.0' +workflow: + id: w + inputs: + - name: api_key + type: secret +${AGENT} + nodes: + - id: n + type: agent + agent_ref: ag + prompt_template: 'use {{inputs.api_key}}' + edges: []`); + expect(err.leaks[0]).toEqual({ + location: 'node `n`.prompt_template', + secret: 'inputs.api_key', + }); + expect(err.code).toBe('secret_interpolation'); + }); + + it('rejects a secret laundered through a context entry (transitive taint, with `via`)', () => { + const err = expectLeak(`schema_version: '1.0' +workflow: + id: w + inputs: + - name: api_key + type: secret + context: + - key: creds + value: 'Bearer {{inputs.api_key}}' +${AGENT} + nodes: + - id: n + type: agent + agent_ref: ag + prompt_template: 'auth {{ctx.creds}}' + edges: []`); + expect(err.leaks[0]).toEqual({ + location: 'node `n`.prompt_template', + secret: 'ctx.creds', + via: 'inputs.api_key', + }); + expect(err.message).toContain('inputs.api_key'); + expect(err.message).toContain('ADR-0029'); + }); + + it('reaches taint through a two-hop, out-of-order context chain (transitive worklist)', () => { + const err = expectLeak(`schema_version: '1.0' +workflow: + id: w + inputs: + - name: s + type: secret + context: + - key: b + value: '{{ctx.a}}' + - key: a + value: '{{inputs.s}}' +${AGENT} + nodes: + - id: n + type: agent + agent_ref: ag + prompt_template: '{{ctx.b}}' + edges: []`); + expect(err.leaks[0]).toEqual({ + location: 'node `n`.prompt_template', + secret: 'ctx.b', + via: 'ctx.a', + }); + }); + + it('rejects a `secrets.*` namespace reference used directly in human-gate text', () => { + const err = expectLeak(`schema_version: '1.0' +workflow: + id: w + nodes: + - id: g + type: human_gate + gate_type: review + message_template: 'token {{secrets.token}}' + edges: []`); + expect(err.leaks[0]).toEqual({ + location: 'node `g`.message_template', + secret: 'secrets.token', + }); + }); + + it('launders a `secrets.*` reference through a context value into text (via the secret store)', () => { + const err = expectLeak(`schema_version: '1.0' +workflow: + id: w + context: + - key: creds + value: 'Bearer {{secrets.token}}' +${AGENT} + nodes: + - id: n + type: agent + agent_ref: ag + prompt_template: 'auth {{ctx.creds}}' + edges: []`); + expect(err.leaks[0]).toEqual({ + location: 'node `n`.prompt_template', + secret: 'ctx.creds', + via: 'secrets.token', + }); + }); + + it('launders a `secrets.*` reference through a non-secret input default into text', () => { + const err = expectLeak(`schema_version: '1.0' +workflow: + id: w + inputs: + - name: reviewer + type: string + default: '{{secrets.token}}' +${AGENT} + nodes: + - id: n + type: agent + agent_ref: ag + prompt_template: '{{inputs.reviewer}}' + edges: []`); + expect(err.leaks[0]).toEqual({ + location: 'node `n`.prompt_template', + secret: 'inputs.reviewer', + via: 'secrets.token', + }); + }); + + it('rejects a secret laundered through a non-secret input default (transitive via the default)', () => { + // A `string` input whose default reads a secret resolves to the secret value at runtime, so a + // prompt that reads that input would leak it — the taint must close over input defaults too. + const err = expectLeak(`schema_version: '1.0' +workflow: + id: w + inputs: + - name: api_key + type: secret + - name: reviewer + type: string + default: 'Bearer {{inputs.api_key}}' +${AGENT} + nodes: + - id: n + type: agent + agent_ref: ag + prompt_template: 'auth {{inputs.reviewer}}' + edges: []`); + expect(err.leaks[0]).toEqual({ + location: 'node `n`.prompt_template', + secret: 'inputs.reviewer', + via: 'inputs.api_key', + }); + }); + + it('rejects a secret in an inline agent `system_prompt` and a node `system_prompt_append`', () => { + const err = expectLeak(`schema_version: '1.0' +workflow: + id: w + inputs: + - name: api_key + type: secret + agents: + - id: ag + name: Ag + model: claude-sonnet-4-6 + provider: anthropic + system_prompt: 'be terse {{inputs.api_key}}' + nodes: + - id: n + type: agent + agent_ref: ag + system_prompt_append: 'and {{inputs.api_key}}' + edges: []`); + const locations = err.leaks.map((leak) => leak.location); + expect(locations).toContain('agent `ag`.system_prompt'); + expect(locations).toContain('node `n`.system_prompt_append'); + }); + + it('launders a secret through TWO chained input defaults (multi-hop transitive)', () => { + const err = expectLeak(`schema_version: '1.0' +workflow: + id: w + inputs: + - name: a + type: secret + - name: b + type: string + default: '{{inputs.a}}' + - name: c + type: string + default: '{{inputs.b}}' +${AGENT} + nodes: + - id: n + type: agent + agent_ref: ag + prompt_template: '{{inputs.c}}' + edges: []`); + expect(err.leaks[0]).toEqual({ + location: 'node `n`.prompt_template', + secret: 'inputs.c', + via: 'inputs.b', + }); + }); + + it('rejects a secret read via a trailing path — taint keys on the symbol, not the path (no via)', () => { + const err = expectLeak(`schema_version: '1.0' +workflow: + id: w + inputs: + - name: api_key + type: secret +${AGENT} + nodes: + - id: n + type: agent + agent_ref: ag + prompt_template: 'token {{inputs.api_key.token}}' + edges: []`); + expect(err.leaks[0]).toEqual({ + location: 'node `n`.prompt_template', + secret: 'inputs.api_key', + }); + }); + + it('rejects a secret interpolated into a human_gate `assignee`', () => { + const err = expectLeak(`schema_version: '1.0' +workflow: + id: w + inputs: + - name: api_key + type: secret + nodes: + - id: g + type: human_gate + gate_type: approval + assignee: '{{inputs.api_key}}' + edges: []`); + expect(err.leaks[0]).toEqual({ location: 'node `g`.assignee', secret: 'inputs.api_key' }); + }); + + it('reports every leaking site — two nodes referencing the same secret yield two leaks', () => { + const err = expectLeak(`schema_version: '1.0' +workflow: + id: w + inputs: + - name: api_key + type: secret +${AGENT} + nodes: + - id: a + type: agent + agent_ref: ag + prompt_template: 'one {{inputs.api_key}}' + - id: b + type: agent + agent_ref: ag + prompt_template: 'two {{inputs.api_key}}' + edges: []`); + expect(err.leaks).toHaveLength(2); + expect(err.leaks.map((leak) => leak.location)).toEqual([ + 'node `a`.prompt_template', + 'node `b`.prompt_template', + ]); + }); +}); + +describe('analyzeSecretTaint — permitted (no leak)', () => { + it('allows a secret to flow into a context entry that is never used in text', () => { + // A secret may feed a credential/header path; it is only rejected when it reaches model/human text. + const wf = parseWorkflow(`schema_version: '1.0' +workflow: + id: w + inputs: + - name: api_key + type: secret + context: + - key: creds + value: '{{inputs.api_key}}' + nodes: + - id: n + type: input + edges: []`); + expect(analyzeSecretTaint(wf)).toEqual([]); + }); + + it('returns no leaks for the canonical (secret-free) pipeline', () => { + const wf = parseWorkflow(`schema_version: '1.0' +workflow: + id: w + inputs: + - name: file_path + type: file_path +${AGENT} + nodes: + - id: n + type: agent + agent_ref: ag + prompt_template: 'review {{inputs.file_path}}' + edges: []`); + expect(analyzeSecretTaint(wf)).toEqual([]); + }); +}); + +describe('analyzePreRunReferences', () => { + it('returns no issues for a clean context (reads inputs/ctx only)', () => { + const wf = parseWorkflow(`schema_version: '1.0' +workflow: + id: w + inputs: + - name: p + type: string + context: + - key: ok + value: '{{inputs.p}}' + nodes: + - id: n + type: input + edges: []`); + expect(analyzePreRunReferences(wf)).toEqual([]); + }); + + it('flags a context value that reads run.outputs (the positive branch, in isolation)', () => { + // Built via the schema directly — parseWorkflow would reject this, so the analyzer is exercised + // here on its own to pin the issue shape it produces. + const wf = WorkflowSchema.parse({ + schema_version: '1.0', + workflow: { + id: 'w', + context: [{ key: 'snapshot', value: '{{run.outputs["x"]}}' }], + nodes: [{ id: 'x', type: 'input' }], + edges: [], + }, + }); + const issues = analyzePreRunReferences(wf); + expect(issues).toHaveLength(1); + expect(issues[0]?.field).toBe('context `snapshot`.value'); + expect(issues[0]?.message).toContain('run.outputs'); + }); + + it('flags an input default that reads run.outputs (defaults also resolve pre-run)', () => { + // parseWorkflow rejects this with a WorkflowValidationError, consistent with the context gate. + let thrown: unknown; + try { + parseWorkflow(`schema_version: '1.0' +workflow: + id: w + inputs: + - name: seeded + type: string + default: 'from {{run.outputs["x"]}}' + nodes: + - id: x + type: input + edges: []`); + } catch (err) { + thrown = err; + } + expect(thrown).toBeInstanceOf(WorkflowValidationError); + if (!(thrown instanceof WorkflowValidationError)) { + throw new Error('expected a WorkflowValidationError'); + } + expect(thrown.issues[0]?.field).toBe('input `seeded`.default'); + expect(thrown.issues[0]?.message).toContain('run.outputs'); + }); +}); diff --git a/packages/core/src/interpolation/analyze.ts b/packages/core/src/interpolation/analyze.ts new file mode 100644 index 00000000..64cc4def --- /dev/null +++ b/packages/core/src/interpolation/analyze.ts @@ -0,0 +1,220 @@ +/** + * Static, parse-time interpolation analyses (1.L2) — no values, no I/O, pure functions over an + * already-validated `Workflow` and the structured references `collectReferences` yields. + * + * - `analyzeSecretTaint` enforces ADR-0029(c): a `secret`-typed input — or anything transitively + * derived from one through a `context` entry *or* an `input` default — must never reach agent/human + * text. An input's *type* alone seeds the taint, so the whole check runs before any secret value is + * fetched. + * - `analyzePreRunReferences` enforces the eager-resolution rule (workflow-yaml-spec.md + * §Context-and-interpolation): a value resolved **before any node runs** — a `context` value or an + * `input` default — may read `{{inputs.*}}`/`{{ctx.*}}` but not `{{run.outputs[…]}}`. + * + * Both name only fields, input names, and context keys — never an authored value — so their findings + * are safe to surface and log. + * + * Scope: this analysis covers the `{{ … }}` template graph only. Secret flow through the JS expression + * fields (`condition`/`transform`/`merge_fn`) and through `run.outputs` is the responsibility of the + * expression sandbox (1.AB) and the run loop (1.M/1.O) — a `transform` that returns a secret cannot be + * caught here because it is not a template. ADR-0029(c)'s "any derived value" spans those layers too. + */ + +import type { Workflow } from '@relavium/shared'; + +import type { SecretLeak, WorkflowIssue } from '../errors.js'; + +import { collectReferences } from './collect.js'; +import { templateReferences, type InterpolationReference } from './references.js'; + +/** + * The transitive taint sets — which input names and context keys carry (or launder) a secret. An + * input is tainted if it is `secret`-typed or its default reads a tainted symbol; a context key is + * tainted if its value reads one. The stored string is the deeper tainted symbol (the "via"), kept + * for a precise, value-free error message; a source secret input has `undefined` (no deeper hop). + */ +interface TaintSets { + readonly inputs: ReadonlyMap; + readonly ctx: ReadonlyMap; +} + +/** + * Find every place a secret reaches agent/human text. Empty when the workflow is clean; the parser + * turns a non-empty result into a `WorkflowSecretLeakError` (rejected at parse). + */ +export function analyzeSecretTaint(workflow: Workflow): readonly SecretLeak[] { + const taint = computeTaint(workflow.workflow); + + const leaks: SecretLeak[] = []; + for (const site of collectReferences(workflow)) { + // Only model/human-visible text is a leak; context values and input defaults are where taint + // legitimately propagates (closed over by `computeTaint`), never themselves a model-bound site. + if (site.category === 'context-value' || site.category === 'input-default') { + continue; + } + for (const ref of site.references) { + const reason = taintReason(ref, taint); + if (reason !== undefined) { + leaks.push(toLeak(site.location, reason, taint)); + } + } + } + return leaks; +} + +/** + * Find a value resolved before any node runs that references a node output. Empty when clean; the + * parser turns a non-empty result into a `WorkflowValidationError` (a field-named parse error). + */ +export function analyzePreRunReferences(workflow: Workflow): readonly WorkflowIssue[] { + const spec = workflow.workflow; + const issues: WorkflowIssue[] = []; + const checkNoNodeOutput = (field: string, text: string): void => { + for (const ref of templateReferences(text)) { + if (ref.kind === 'node') { + issues.push({ + field, + message: `cannot reference \`run.outputs[…]\` — this is resolved before any node runs`, + }); + return; // one issue per site is enough to fail the parse + } + } + }; + for (const entry of spec.context ?? []) { + checkNoNodeOutput(`context \`${entry.key}\`.value`, entry.value); + } + for (const input of spec.inputs ?? []) { + // Only a string default is a template. A `{{ … }}` nested in a STRUCTURED default + // (`default: { token: '{{secrets.x}}' }`) is not scanned — it is never re-interpolated either + // (`resolveTemplate` is single-pass, so `| json` emits the literal `{{…}}`, not a resolved value), + // so this is a deferral to the 1.M typed-input layer, not a gap that leaks. + if (typeof input.default === 'string') { + checkNoNodeOutput(`input \`${input.name}\`.default`, input.default); + } + } + return issues; +} + +/** Mutable graph state for the linear taint pass: tainted symbols, reverse edges, and the worklist. */ +interface TaintGraph { + readonly tainted: Map; // symbol id (`inputs.`/`ctx.`) → deeper "via" + readonly dependents: Map; // target symbol → the fields that read it + readonly queue: string[]; +} + +/** Mark a symbol tainted (idempotent) and enqueue it for propagation. */ +function seedTaint(graph: TaintGraph, id: string, via: string | undefined): void { + if (!graph.tainted.has(id)) { + graph.tainted.set(id, via); + graph.queue.push(id); + } +} + +/** Record that `dependent` reads `target`, so tainting `target` later taints `dependent`. */ +function addEdge(graph: TaintGraph, target: string, dependent: string): void { + const list = graph.dependents.get(target); + if (list === undefined) { + graph.dependents.set(target, [dependent]); + } else { + list.push(dependent); + } +} + +/** Scan one pre-run field's references: a `secrets.*` read seeds it; an inputs/ctx read adds an edge. */ +function scanField(graph: TaintGraph, id: string, text: string | undefined): void { + if (text === undefined) { + return; + } + for (const ref of templateReferences(text)) { + if (ref.kind === 'secrets') { + seedTaint(graph, id, `secrets.${ref.identifier}`); // reads a secret store directly → a source + } else if (ref.kind === 'inputs') { + addEdge(graph, `inputs.${ref.identifier}`, id); + } else if (ref.kind === 'ctx') { + addEdge(graph, `ctx.${ref.identifier}`, id); + } + } +} + +/** Drain the worklist: tainting a target taints every field that reads it (via = the target). */ +function propagateTaint(graph: TaintGraph): void { + while (graph.queue.length > 0) { + const target = graph.queue.pop(); + if (target === undefined) { + break; + } + for (const dependent of graph.dependents.get(target) ?? []) { + seedTaint(graph, dependent, target); + } + } +} + +/** Split the flat `symbol → via` map back into the per-namespace taint sets the leak check reads. */ +function projectTaint(tainted: ReadonlyMap): TaintSets { + const inputs = new Map(); + const ctx = new Map(); + for (const [id, via] of tainted) { + if (id.startsWith('inputs.')) { + inputs.set(id.slice('inputs.'.length), via); + } else if (id.startsWith('ctx.')) { + ctx.set(id.slice('ctx.'.length), via); + } + } + return { inputs, ctx }; +} + +/** + * Compute the taint sets in **linear** time. Each pre-run field (an `input` default or a `context` + * value) is parsed for its references exactly once; a secret source (a `secret`-typed input, or a + * field reading `{{secrets.*}}`) seeds a worklist, and taint propagates along reverse-dependency edges + * until the queue drains — O(symbols + references), with no per-round re-parsing. Deliberately not a + * rescan-to-fixpoint loop: that was O(N²) over the entry count and let a small reversed-laundering + * chain YAML stall the synchronous parse gate. + */ +function computeTaint(spec: Workflow['workflow']): TaintSets { + const graph: TaintGraph = { tainted: new Map(), dependents: new Map(), queue: [] }; + for (const input of spec.inputs ?? []) { + if (input.type === 'secret') { + seedTaint(graph, `inputs.${input.name}`, undefined); // a source secret — no deeper "via" + } + // Only a string default carries a template; a `{{ … }}` inside a STRUCTURED default is not scanned + // (and never re-interpolated), so it cannot launder a secret — deferred to the 1.M typed-input layer. + const fallback = typeof input.default === 'string' ? input.default : undefined; + scanField(graph, `inputs.${input.name}`, fallback); + } + for (const entry of spec.context ?? []) { + scanField(graph, `ctx.${entry.key}`, entry.value); + } + propagateTaint(graph); + return projectTaint(graph.tainted); +} + +/** The tainted symbol a reference reads, or `undefined` if it is clean. Names only — never a value. */ +function taintReason(ref: InterpolationReference, taint: TaintSets): string | undefined { + if (ref.kind === 'inputs' && taint.inputs.has(ref.identifier)) { + return `inputs.${ref.identifier}`; + } + if (ref.kind === 'ctx' && taint.ctx.has(ref.identifier)) { + return `ctx.${ref.identifier}`; + } + if (ref.kind === 'secrets') { + return `secrets.${ref.identifier}`; + } + return undefined; +} + +/** Build a leak finding, attaching the deeper "via" symbol when the taint was laundered. */ +function toLeak(location: string, secret: string, taint: TaintSets): SecretLeak { + const via = viaOf(secret, taint); + return { location, secret, ...(via === undefined ? {} : { via }) }; +} + +/** The deeper tainted symbol a laundered input/ctx symbol came from (a source secret has none). */ +function viaOf(secret: string, taint: TaintSets): string | undefined { + if (secret.startsWith('ctx.')) { + return taint.ctx.get(secret.slice('ctx.'.length)); + } + if (secret.startsWith('inputs.')) { + return taint.inputs.get(secret.slice('inputs.'.length)); + } + return undefined; // a direct `secrets.*` reference +} diff --git a/packages/core/src/interpolation/collect.test.ts b/packages/core/src/interpolation/collect.test.ts new file mode 100644 index 00000000..3ea3e656 --- /dev/null +++ b/packages/core/src/interpolation/collect.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it } from 'vitest'; + +import { parseWorkflow } from '../parser.js'; + +import { collectReferences } from './collect.js'; + +describe('collectReferences — site categories', () => { + it('collects a human_gate `assignee` and `message_template` as node-text', () => { + const wf = parseWorkflow(`schema_version: '1.0' +workflow: + id: w + inputs: + - name: who + type: string + nodes: + - id: g + type: human_gate + gate_type: review + assignee: '{{inputs.who}}' + message_template: 'hi {{inputs.who}}' + edges: []`); + const byLocation = new Map(collectReferences(wf).map((s) => [s.location, s.category])); + expect(byLocation.get('node `g`.assignee')).toBe('node-text'); + expect(byLocation.get('node `g`.message_template')).toBe('node-text'); + }); + + it('tags context values, input defaults, and inline agent system prompts by category', () => { + const wf = parseWorkflow(`schema_version: '1.0' +workflow: + id: w + inputs: + - name: p + type: string + default: 'fallback {{inputs.p}}' + context: + - key: c + value: '{{inputs.p}}' + agents: + - id: ag + name: Ag + model: claude-sonnet-4-6 + provider: anthropic + system_prompt: 'sys {{ctx.c}}' + nodes: + - id: n + type: input + edges: []`); + const byLocation = new Map(collectReferences(wf).map((s) => [s.location, s.category])); + expect(byLocation.get('context `c`.value')).toBe('context-value'); + expect(byLocation.get('input `p`.default')).toBe('input-default'); + expect(byLocation.get('agent `ag`.system_prompt')).toBe('agent-text'); + }); +}); diff --git a/packages/core/src/interpolation/collect.ts b/packages/core/src/interpolation/collect.ts index 1007c596..e1b6e64b 100644 --- a/packages/core/src/interpolation/collect.ts +++ b/packages/core/src/interpolation/collect.ts @@ -14,10 +14,20 @@ import type { Workflow } from '@relavium/shared'; import { parseTemplate, type InterpolationReference, type TemplateSegment } from './references.js'; +/** + * What kind of field a reference site is — the distinction the secret-taint analyzer (1.L2) needs. + * `agent-text`/`node-text` are model/human-visible and so are leak-checked; `context-value` is where + * taint *propagates* (not itself a leak) and `input-default` is a fallback value, neither of which is + * sent to a model. The DAG builder (1.M) also reads this to know which sites carry run data. + */ +export type ReferenceSiteCategory = 'context-value' | 'input-default' | 'agent-text' | 'node-text'; + /** One field that carries at least one `{{ … }}` reference. */ export interface ReferenceSite { /** A human field locator, e.g. ``node `synthesize-report`.prompt_template``. */ readonly location: string; + /** Which kind of field this is — text (leak-checked) vs context/default (taint-propagating). */ + readonly category: ReferenceSiteCategory; /** The ordered literal/reference segments of the field's value. */ readonly segments: readonly TemplateSegment[]; /** Just the references at this site, in order. */ @@ -35,7 +45,11 @@ export interface ReferenceSite { type WorkflowNode = Workflow['workflow']['nodes'][number]; /** Build a ReferenceSite for `text` if it contains at least one interpolation reference. */ -function buildSite(location: string, text: string): ReferenceSite | undefined { +function buildSite( + location: string, + category: ReferenceSiteCategory, + text: string, +): ReferenceSite | undefined { const segments = parseTemplate(text); const references: InterpolationReference[] = []; for (const segment of segments) { @@ -43,15 +57,15 @@ function buildSite(location: string, text: string): ReferenceSite | undefined { references.push(segment.reference); } } - return references.length > 0 ? { location, segments, references } : undefined; + return references.length > 0 ? { location, category, segments, references } : undefined; } -/** Collect reference sites from template fields on a single workflow node. */ +/** Collect reference sites from template fields on a single workflow node — all model/human text. */ function collectNodeSites(node: WorkflowNode): ReferenceSite[] { const sites: ReferenceSite[] = []; const addFieldSite = (label: string, value: string | undefined): void => { if (value !== undefined) { - const site = buildSite(label, value); + const site = buildSite(label, 'node-text', value); if (site !== undefined) sites.push(site); } }; @@ -69,29 +83,29 @@ export function collectReferences(workflow: Workflow): readonly ReferenceSite[] const sites: ReferenceSite[] = []; const spec = workflow.workflow; - const push = (location: string, text: string): void => { - const site = buildSite(location, text); + const push = (location: string, category: ReferenceSiteCategory, text: string): void => { + const site = buildSite(location, category, text); if (site !== undefined) sites.push(site); }; - // TODO(1.M): workflow-yaml-spec.md §Context-and-interpolation forbids `{{run.outputs[...]}}` in - // context values (context is eagerly evaluated before the run; node outputs are unavailable). - // Enforcing this here would require importing parseTemplate, coupling the collector to a semantic - // constraint that the DAG builder (1.M) is better placed to enforce (it wires data-dependency - // edges and can produce a richer error). Until then, a context entry that references a node - // output is collected with kind:'node' and will be caught by the DAG builder when it finds no - // satisfying edge for it. See the pinned test in parser.test.ts ("permits a context value …"). + // A `{{run.outputs[…]}}` reference in a context value is rejected at parse by + // `analyzePreRunReferences` (1.L2) — context is eagerly resolved before any node runs, so no node + // output exists yet (workflow-yaml-spec.md §Context-and-interpolation). The collector stays a pure + // structural view: it records the site (with kind:'node'); the analyzer reads it and raises the + // field-named parse error. for (const entry of spec.context ?? []) { - push(`context \`${entry.key}\`.value`, entry.value); + push(`context \`${entry.key}\`.value`, 'context-value', entry.value); } for (const input of spec.inputs ?? []) { if (typeof input.default === 'string') { - push(`input \`${input.name}\`.default`, input.default); + push(`input \`${input.name}\`.default`, 'input-default', input.default); } } for (const agent of spec.agents ?? []) { + // TODO(1.M): a `$ref` agent's `system_prompt` lives in another file; once 1.M resolves the ref, + // the resolved prompt must be re-run through `analyzeSecretTaint` so a secret cannot hide behind it. if (!('$ref' in agent)) { - push(`agent \`${agent.id}\`.system_prompt`, agent.system_prompt); + push(`agent \`${agent.id}\`.system_prompt`, 'agent-text', agent.system_prompt); } } for (const node of spec.nodes) { diff --git a/packages/core/src/interpolation/filters.test.ts b/packages/core/src/interpolation/filters.test.ts new file mode 100644 index 00000000..bc320065 --- /dev/null +++ b/packages/core/src/interpolation/filters.test.ts @@ -0,0 +1,170 @@ +import { describe, expect, it } from 'vitest'; + +import { InterpolationError } from '../errors.js'; + +import { filterFn } from './filters.js'; +import type { ResolverCapabilities } from './scope.js'; +import type { FilterArg, InterpolationReference } from './references.js'; + +/** Apply a filter by name directly (the path resolve.ts takes), returning its result or throwing. */ +function runFilter( + name: string, + value: unknown, + args: readonly FilterArg[] = [], + caps: ResolverCapabilities = {}, +): unknown { + const ref: InterpolationReference = { + kind: 'inputs', + identifier: 'x', + path: '', + filters: [{ name, args }], + raw: `{{inputs.x | ${name}}}`, + }; + return filterFn({ name, args }, ref)(value, args, caps, ref); +} + +const str = (value: string): FilterArg => ({ type: 'string', value }); + +describe('filterFn — registry lookup safety', () => { + it.each(['toString', 'constructor', '__proto__', 'valueOf', 'hasOwnProperty', 'isPrototypeOf'])( + 'rejects the inherited registry member %j as unknown_filter (no prototype invocation)', + (name) => { + let thrown: unknown; + try { + runFilter(name, 'v'); + } catch (err) { + thrown = err; + } + expect(thrown).toBeInstanceOf(InterpolationError); + if (!(thrown instanceof InterpolationError)) { + throw new Error('expected an InterpolationError'); + } + expect(thrown.code).toBe('unknown_filter'); + }, + ); + + it('rejects a plain unknown filter name', () => { + expect(() => runFilter('nope', 'v')).toThrow(InterpolationError); + }); +}); + +describe('json filter', () => { + it('serializes a scalar and an object as pretty JSON', () => { + expect(runFilter('json', 0)).toBe('0'); + expect(runFilter('json', { a: 1 })).toBe(JSON.stringify({ a: 1 }, null, 2)); + }); +}); + +describe('length filter', () => { + it('counts UTF-16 code units for a string (matching the docblock, not codepoints)', () => { + expect(runFilter('length', 'abcd')).toBe(4); + expect(runFilter('length', '👍ab')).toBe(4); // 👍 is two UTF-16 code units + expect(runFilter('length', '')).toBe(0); + }); + + it('counts array items and object own keys', () => { + expect(runFilter('length', [1, 2, 3])).toBe(3); + expect(runFilter('length', [])).toBe(0); + expect(runFilter('length', { a: 1, b: 2 })).toBe(2); + }); + + it('rejects a non-countable value', () => { + expect(() => runFilter('length', 5)).toThrow(InterpolationError); + }); +}); + +describe('default filter', () => { + it('passes a falsy-but-present value through (0 / false / empty string), rescuing only null/undefined', () => { + expect(runFilter('default', 0, [str('FB')])).toBe(0); + expect(runFilter('default', false, [str('FB')])).toBe(false); + expect(runFilter('default', '', [str('FB')])).toBe(''); + expect(runFilter('default', null, [str('FB')])).toBe('FB'); + expect(runFilter('default', undefined, [str('FB')])).toBe('FB'); + }); + + it('requires exactly one argument', () => { + expect(() => runFilter('default', 'v', [])).toThrow(InterpolationError); + }); +}); + +describe('read_file filter', () => { + it('reads via the host capability and forwards the AbortSignal', async () => { + const controller = new AbortController(); + let received: unknown; + const caps: ResolverCapabilities = { + readFile: (path, signal) => { + received = signal; + return `FILE(${path})`; + }, + }; + const ref: InterpolationReference = { + kind: 'inputs', + identifier: 'x', + path: '', + filters: [{ name: 'read_file', args: [] }], + raw: '{{inputs.x | read_file}}', + }; + const out = await filterFn({ name: 'read_file', args: [] }, ref)( + 'a.ts', + [], + caps, + ref, + controller.signal, + ); + expect(out).toBe('FILE(a.ts)'); + expect(received).toBe(controller.signal); + }); + + it('fails typed when no host reader is provided', async () => { + await expect(runFilter('read_file', 'a.ts')).rejects.toMatchObject({ + code: 'read_file_unavailable', + }); + }); + + it('classifies a host AbortError as `aborted`, not `read_file_failed`', async () => { + const abortErr = new Error('cancelled'); + abortErr.name = 'AbortError'; + await expect( + runFilter('read_file', 'a.ts', [], { + readFile: () => { + throw abortErr; + }, + }), + ).rejects.toMatchObject({ code: 'aborted' }); + }); + + it('classifies a non-abort host error as `read_file_failed`', async () => { + await expect( + runFilter('read_file', 'a.ts', [], { + readFile: () => { + throw new Error('ENOENT'); + }, + }), + ).rejects.toMatchObject({ code: 'read_file_failed' }); + }); + + it('classifies a plain host error as `aborted` when the run signal already fired (signal operand)', async () => { + const controller = new AbortController(); + controller.abort(); + const ref: InterpolationReference = { + kind: 'inputs', + identifier: 'x', + path: '', + filters: [{ name: 'read_file', args: [] }], + raw: '{{inputs.x | read_file}}', + }; + await expect( + filterFn({ name: 'read_file', args: [] }, ref)( + 'a.ts', + [], + { + readFile: () => { + throw new Error('whatever'); // a plain error, but the signal is what makes it an abort + }, + }, + ref, + controller.signal, + ), + ).rejects.toMatchObject({ code: 'aborted' }); + }); +}); diff --git a/packages/core/src/interpolation/filters.ts b/packages/core/src/interpolation/filters.ts new file mode 100644 index 00000000..880c8fd1 --- /dev/null +++ b/packages/core/src/interpolation/filters.ts @@ -0,0 +1,165 @@ +/** + * The pipe-filter registry (1.L2) — `| json`, `| length`, `| default(…)`, `| read_file` + * (workflow-yaml-spec.md §Context-and-interpolation). Each filter is a pure transform over the value + * resolved so far, except `read_file`, which calls the host-injected `readFile` capability so the + * engine never imports `node:fs` (CLAUDE.md rule 5). A bad filter name, wrong arity, or wrong input + * type fails with a typed, secret-free {@link InterpolationError} — never a thrown string or a crash. + * + * Filters are deterministic given their input (and, for `read_file`, a deterministic host reader), so + * a re-resolved scope is identical — the property the checkpoint/idempotency model (1.R) relies on. + */ + +import type { AbortSignalLike } from '@relavium/shared'; + +import { InterpolationError } from '../errors.js'; + +import type { ResolverCapabilities } from './scope.js'; +import type { FilterArg, InterpolationReference, PipeFilter } from './references.js'; + +/** + * A filter: the value so far, its parsed args, the host capabilities, the source ref (for errors), and + * an optional `AbortSignal` (only `read_file` honors it). The result is `unknown` (which already + * subsumes `Promise`); the resolver `await`s it, so a filter may be sync (`json`/`length`/ + * `default`) or async (`read_file`). + */ +export type FilterFn = ( + value: unknown, + args: readonly FilterArg[], + caps: ResolverCapabilities, + ref: InterpolationReference, + signal?: AbortSignalLike, +) => unknown; + +const FILTERS: Readonly> = { + /** Serialize the value as pretty JSON — the supported way to embed an object/array in text. */ + json: (value, args, _caps, ref) => { + requireArity('json', args, 0, ref); + let serialized: string | undefined; + try { + serialized = JSON.stringify(value, null, 2); + } catch (err) { + // A circular structure or a BigInt makes JSON.stringify throw a raw TypeError whose message can + // embed value-shape detail — keep it on `cause` for logs and surface a typed, secret-free error. + throw new InterpolationError( + 'unserializable', + `\`${ref.raw}\` could not be serialized as JSON`, + { + location: ref.raw, + cause: err, + }, + ); + } + if (serialized === undefined) { + // `undefined`, a function, or a lone symbol — there is nothing meaningful to embed. + throw filterType('json', 'a JSON-serializable value', ref); + } + return serialized; + }, + + /** The count of a string's UTF-16 code units, an array's items, or an object's own keys. */ + length: (value, args, _caps, ref) => { + requireArity('length', args, 0, ref); + if (typeof value === 'string' || Array.isArray(value)) { + return value.length; + } + if (typeof value === 'object' && value !== null) { + return Object.keys(value).length; + } + throw filterType('length', 'a string, list, or object', ref); + }, + + /** Substitute a literal when the value resolved to nothing (`undefined`/`null`); else pass it through. */ + default: (value, args, _caps, ref) => { + requireArity('default', args, 1, ref); + if (value === undefined || value === null) { + return (args[0] as FilterArg).value; + } + return value; + }, + + /** Read a workspace file's text via the host `readFile` capability (the engine never touches disk). */ + read_file: async (value, args, caps, ref, signal) => { + requireArity('read_file', args, 0, ref); + if (typeof value !== 'string') { + throw filterType('read_file', 'a file path string', ref); + } + if (caps.readFile === undefined) { + throw new InterpolationError( + 'read_file_unavailable', + `the \`read_file\` filter needs a host file reader, which this run did not provide`, + { location: ref.raw }, + ); + } + try { + return await caps.readFile(value, signal); + } catch (err) { + if (signal?.aborted === true || isAbortError(err)) { + // A cancelled read is not an I/O failure — surface it as the run-wide abort, not read_file_failed. + throw new InterpolationError('aborted', `\`read_file\` was aborted`, { + location: ref.raw, + cause: err, + }); + } + // The authored path / host error (which may carry an absolute path) stays on `cause` for logs; + // the user-facing message names only the reference, never the path value. + throw new InterpolationError('read_file_failed', `\`read_file\` could not read the file`, { + location: ref.raw, + cause: err, + }); + } + }, +}; + +/** Look up a filter by name, or throw a typed `unknown_filter` error naming the offending reference. */ +export function filterFn(filter: PipeFilter, ref: InterpolationReference): FilterFn { + // `Object.hasOwn` so an inherited member of the registry object (`toString`, `constructor`, + // `__proto__`) is never mistaken for a filter — a bare `FILTERS[name]` would return + // `Object.prototype.toString` for `| toString` and then invoke it. + const fn = Object.hasOwn(FILTERS, filter.name) ? FILTERS[filter.name] : undefined; + if (fn === undefined) { + throw new InterpolationError('unknown_filter', `unknown filter \`${filter.name}\``, { + location: ref.raw, + }); + } + return fn; +} + +function requireArity( + name: string, + args: readonly FilterArg[], + arity: number, + ref: InterpolationReference, +): void { + if (args.length !== arity) { + const expected = arity === 1 ? '1 argument' : `${arity} arguments`; + throw new InterpolationError( + 'filter_arity', + `filter \`${name}\` expects ${expected}, got ${args.length}`, + { location: ref.raw }, + ); + } +} + +function filterType( + name: string, + expected: string, + ref: InterpolationReference, +): InterpolationError { + return new InterpolationError('filter_type', `filter \`${name}\` expects ${expected}`, { + location: ref.raw, + }); +} + +/** + * Whether a thrown value is a standard cancellation (`AbortError` from a host reader honoring a signal). + * Checks the `name` structurally rather than via `instanceof Error`, because in the Tauri WebView a + * `DOMException('…','AbortError')` is not an `Error` instance (CLAUDE.md rule 5 — the engine runs there). + */ +function isAbortError(err: unknown): boolean { + return ( + typeof err === 'object' && + err !== null && + 'name' in err && + (err as { name?: unknown }).name === 'AbortError' + ); +} diff --git a/packages/core/src/interpolation/path.test.ts b/packages/core/src/interpolation/path.test.ts new file mode 100644 index 00000000..8bc755c2 --- /dev/null +++ b/packages/core/src/interpolation/path.test.ts @@ -0,0 +1,110 @@ +import { describe, expect, it } from 'vitest'; + +import { InterpolationError } from '../errors.js'; + +import { getByPath } from './path.js'; + +describe('getByPath', () => { + const obj = { + score: 7, + issues: [{ line: 1 }, { line: 2 }], + 'a-b': 'dashed', + nested: { deep: { value: 'x' } }, + }; + + it('returns the value unchanged for an empty path', () => { + expect(getByPath(obj, '')).toBe(obj); + }); + + it('reads a dotted property', () => { + expect(getByPath(obj, '.score')).toBe(7); + }); + + it('reads a numeric array index then a property', () => { + expect(getByPath(obj, '.issues[0].line')).toBe(1); + expect(getByPath(obj, '.issues[1].line')).toBe(2); + }); + + it('reads a quoted bracket key (single or double quotes)', () => { + expect(getByPath(obj, '["a-b"]')).toBe('dashed'); + expect(getByPath(obj, "['a-b']")).toBe('dashed'); + }); + + it('walks a deep chain', () => { + expect(getByPath(obj, '.nested.deep.value')).toBe('x'); + }); + + it('returns undefined for a missing property (no throw)', () => { + expect(getByPath(obj, '.nope')).toBeUndefined(); + expect(getByPath(obj, '.nested.missing.deeper')).toBeUndefined(); + }); + + it('returns undefined when indexing a non-array or keying a non-object', () => { + expect(getByPath(obj, '.score[0]')).toBeUndefined(); // index into a number + expect(getByPath(obj, '.issues.line')).toBeUndefined(); // named key on an array + }); + + it('returns undefined when hopping off null/undefined mid-chain', () => { + expect(getByPath({ a: null }, '.a.b')).toBeUndefined(); + expect(getByPath(undefined, '.a')).toBeUndefined(); + }); + + it('reads a quoted key that itself contains a `]` (scanner is quote-aware, not first-`]`)', () => { + expect(getByPath({ 'weird]key': 'v' }, '["weird]key"]')).toBe('v'); + expect(getByPath({ scan: { 'a]b': 1 } }, '.scan["a]b"]')).toBe(1); + }); + + it('tolerates whitespace inside brackets, for both quoted keys and numeric indices', () => { + expect(getByPath({ k: 'v' }, '[ "k" ]')).toBe('v'); + expect(getByPath([10, 20], '[ 1 ]')).toBe(20); + }); + + it('does not read a polluted Array.prototype index (own-index guard)', () => { + Reflect.set(Array.prototype, 0, 'POLLUTED'); + try { + expect(getByPath([], '[0]')).toBeUndefined(); // empty array → own index absent → undefined + expect(getByPath([{ a: 1 }], '[0].a')).toBe(1); // a real own index still resolves + } finally { + Reflect.deleteProperty(Array.prototype, 0); + } + }); + + it('rejects a huge (non-safe-integer) or negative numeric index as invalid_path', () => { + for (const bad of ['[99999999999999999999]', '[-1]']) { + expect(() => getByPath([1, 2, 3], bad)).toThrow(InterpolationError); + } + }); + + it('returns undefined for an inherited prototype member (own-property guard)', () => { + // Without the guard these would resolve live Object.prototype members rather than undefined. + for (const proto of [ + '.toString', + '.constructor', + '.__proto__', + '.hasOwnProperty', + '.valueOf', + ]) { + expect(getByPath({ a: 1 }, proto)).toBeUndefined(); + } + // An OWN property that happens to shadow a prototype name still resolves. + expect(getByPath({ toString: 'mine' }, '.toString')).toBe('mine'); + }); + + it.each(['..score', '.', '[0', '[nope]', '.issues[]', 'score', '["k', '["k"x]'])( + 'throws InterpolationError(invalid_path) for the malformed path %j', + (bad) => { + let thrown: unknown; + try { + getByPath(obj, bad, '{{run.outputs["x"]}}'); + } catch (err) { + thrown = err; + } + expect(thrown).toBeInstanceOf(InterpolationError); + if (!(thrown instanceof InterpolationError)) { + throw new Error('expected getByPath to throw InterpolationError'); + } + expect(thrown.code).toBe('invalid_path'); + expect(thrown.location).toBe('{{run.outputs["x"]}}'); + }, + ); +}); diff --git a/packages/core/src/interpolation/path.ts b/packages/core/src/interpolation/path.ts new file mode 100644 index 00000000..7277347b --- /dev/null +++ b/packages/core/src/interpolation/path.ts @@ -0,0 +1,153 @@ +/** + * Safe property/index access for an interpolation reference's trailing `path` (1.L2). + * + * A reference's head resolves to a run-scope value; its `path` (e.g. `.score`, `.issues[0].line`, + * `["a-b"]`) then navigates into that value. The navigation is a tiny char scanner — **never `eval` + * or `new Function`** — that supports dotted property access, numeric array indices, and quoted + * bracket keys, the shapes the lexer (`references.ts`) emits. A missing hop yields `undefined` (the + * resolver decides whether that is an error or a `| default(…)` case); a *malformed* path throws a + * typed {@link InterpolationError} (`invalid_path`). + */ + +import { InterpolationError } from '../errors.js'; + +/** One navigation step: an object property/quoted key (string) or an array index (number). */ +type PathStep = { readonly key: string } | { readonly index: number }; + +const IDENT_CHAR = /[A-Za-z0-9_-]/; +const WHITESPACE = /\s/; +const INTEGER = /^-?\d+$/; + +/** + * Navigate `value` by `path` (verbatim from the lexer, incl. its leading `.`). An empty path returns + * `value` unchanged; a hop off `undefined`/`null` or a missing property returns `undefined`. + * @throws {InterpolationError} `invalid_path` when `path` is syntactically malformed. + */ +export function getByPath(value: unknown, path: string, location?: string): unknown { + let current = value; + for (const step of parsePath(path, location)) { + if (current === undefined || current === null) { + return undefined; + } + current = stepInto(current, step); + } + return current; +} + +function stepInto(current: unknown, step: PathStep): unknown { + if ('index' in step) { + if (!Array.isArray(current)) { + return undefined; + } + // Own index only — `Object.hasOwn` keeps a polluted `Array.prototype[i]` from leaking through a + // sparse/empty array, symmetric with the object-property branch below. + return Object.hasOwn(current, step.index) + ? (current as readonly unknown[])[step.index] + : undefined; + } + // A plain object's OWN property only — arrays expose just numeric indices here, and an own-property + // guard keeps a missing key returning `undefined` instead of reaching an inherited prototype member + // (`.toString`, `.constructor`, `.__proto__`), which would both break the contract and be unsafe. + if (typeof current === 'object' && current !== null && !Array.isArray(current)) { + return Object.hasOwn(current, step.key) + ? (current as Record)[step.key] + : undefined; + } + return undefined; +} + +function parsePath(path: string, location: string | undefined): PathStep[] { + const steps: PathStep[] = []; + let i = 0; + while (i < path.length) { + const ch = path[i]; + if (ch === '.') { + i = readDotProp(path, i + 1, steps, location); + } else if (ch === '[') { + i = readBracket(path, i, steps, location); + } else { + throw invalidPath(path, location); + } + } + return steps; +} + +/** Read a `.name` property starting just past the dot; returns the next index. */ +function readDotProp( + path: string, + from: number, + steps: PathStep[], + location: string | undefined, +): number { + let i = from; + let name = ''; + while (i < path.length && IDENT_CHAR.test(path[i] as string)) { + name += path[i]; + i += 1; + } + if (name === '') { + throw invalidPath(path, location); + } + steps.push({ key: name }); + return i; +} + +/** Read a `[…]` access (numeric index or quoted key) starting at the `[`; returns the next index. */ +function readBracket( + path: string, + from: number, + steps: PathStep[], + location: string | undefined, +): number { + let i = from + 1; + while (i < path.length && WHITESPACE.test(path[i] as string)) { + i += 1; + } + const opener = path[i]; + // A quoted key: scan to the matching closing quote so a `]` *inside* the key (e.g. `["weird]key"]`) + // does not prematurely end the bracket — `indexOf(']')` cannot do that. (Backslash escapes inside a + // quoted key are not honored here; the lexer's `findClose` preserves them, so such an exotic key + // fails safely with `invalid_path` rather than resolving — acceptable for v1.0.) + if (opener === '"' || opener === "'") { + i += 1; + let key = ''; + while (i < path.length && path[i] !== opener) { + key += path[i]; + i += 1; + } + if (path[i] !== opener) { + throw invalidPath(path, location); // unterminated quote + } + i += 1; // past the closing quote + while (i < path.length && WHITESPACE.test(path[i] as string)) { + i += 1; + } + if (path[i] !== ']') { + throw invalidPath(path, location); + } + steps.push({ key }); + return i + 1; + } + // A numeric index: the first `]` after the `[` closes it (a number has no quotes/brackets inside). + const close = path.indexOf(']', from + 1); + if (close === -1) { + throw invalidPath(path, location); + } + const inner = path.slice(from + 1, close).trim(); + if (INTEGER.test(inner)) { + const index = Number(inner); + if (!Number.isSafeInteger(index) || index < 0) { + // A huge (non-safe-integer) or negative index is malformed, not a benign out-of-bounds miss. + throw invalidPath(path, location); + } + steps.push({ index }); + return close + 1; + } + throw invalidPath(path, location); +} + +function invalidPath(path: string, location: string | undefined): InterpolationError { + return new InterpolationError('invalid_path', `invalid property access \`${path}\``, { + ...(location === undefined ? {} : { location }), + }); +} diff --git a/packages/core/src/interpolation/references.test.ts b/packages/core/src/interpolation/references.test.ts index bee62817..925cdccf 100644 --- a/packages/core/src/interpolation/references.test.ts +++ b/packages/core/src/interpolation/references.test.ts @@ -144,6 +144,16 @@ describe('parseTemplate', () => { expect(ref.filters).toEqual([{ name: 'trim', args: [] }]); }); + it('carries a non-identifier filter name verbatim (the resolver later rejects it)', () => { + const ref = refOf(parseTemplate('{{inputs.x | 9bad}}')[0]); + expect(ref.filters).toEqual([{ name: '9bad', args: [] }]); + }); + + it('drops an empty (trailing-comma) filter argument piece', () => { + const ref = refOf(parseTemplate('{{inputs.x | f(a,)}}')[0]); + expect(ref.filters).toEqual([{ name: 'f', args: [{ type: 'string', value: 'a' }] }]); + }); + it('does not let a literal `}}` inside a quoted argument truncate the reference', () => { const segments = parseTemplate('{{inputs.x | default("}}")}} tail'); expect(segments).toEqual([ diff --git a/packages/core/src/interpolation/references.ts b/packages/core/src/interpolation/references.ts index 7809fce9..344f0a32 100644 --- a/packages/core/src/interpolation/references.ts +++ b/packages/core/src/interpolation/references.ts @@ -3,16 +3,18 @@ * * An authored template field (a prompt, a context value, a gate message) may carry `{{ … }}` * occurrences. 1.L turns each into a typed reference for the DAG builder (1.M) — it does NOT - * evaluate anything: the run-scope lookup, the pipe-filter registry, the eager-once snapshot, and - * the secret-taint rejection all belong to the runtime resolver (1.L2) and the JS sandbox (1.AB). - * This module is a pure lexer: text in, structured segments out. It reads no files, touches no - * environment, and holds no state. + * evaluate anything. The run-scope lookup, the pipe-filter registry, and the eager-once snapshot + * belong to the runtime resolver (1.L2); the secret-taint rejection is a parse-time **static** gate + * (1.L2, run in the parser after schema validation, never at runtime); and `condition`/`transform`/ + * `merge_fn` belong to the JS sandbox (1.AB). This module is a pure lexer: text in, structured + * segments out. It reads no files, touches no environment, and holds no state. * - * The four authored namespaces (workflow-yaml-spec.md §Context-and-interpolation): + * The three authored namespaces (workflow-yaml-spec.md §Context-and-interpolation): * - `{{ inputs. }}` → kind `inputs` * - `{{ ctx. }}` → kind `ctx` * - `{{ run.outputs[""] }}` → kind `node` (the roadmap's informal `{{ node.output }}`) - * - `{{ secrets. }}` → kind `secrets` + * The lexer additionally recognizes `{{ secrets. }}` → kind `secrets` ONLY so the resolver and + * the taint gate can reject it with a precise typed error — it is not an authored v1.0 namespace. * Anything else is carried as `unknown` (the resolver, not this lexer, judges validity). */ diff --git a/packages/core/src/interpolation/resolve.test.ts b/packages/core/src/interpolation/resolve.test.ts new file mode 100644 index 00000000..9e39f956 --- /dev/null +++ b/packages/core/src/interpolation/resolve.test.ts @@ -0,0 +1,439 @@ +import { describe, expect, it } from 'vitest'; + +import { InterpolationError } from '../errors.js'; +import { parseWorkflow } from '../parser.js'; + +import { resolveContext, resolveTemplate } from './resolve.js'; +import type { ResolverCapabilities, RunScope } from './scope.js'; + +function scope(over: Partial = {}): RunScope { + return { inputs: {}, ctx: {}, outputs: {}, ...over }; +} + +/** Resolve `text`, asserting it throws an InterpolationError with the given code; returns the error. */ +async function expectCode( + text: string, + s: RunScope, + code: string, + caps?: ResolverCapabilities, + signal?: AbortSignal, +): Promise { + try { + await resolveTemplate(text, s, caps, signal); + } catch (err) { + if (!(err instanceof InterpolationError)) { + throw err; // an unexpected error type — surface it rather than mis-narrowing + } + expect(err.code).toBe(code); + return err; + } + throw new Error(`expected resolveTemplate to throw ${code}`); +} + +describe('resolveTemplate — happy paths', () => { + it('passes a literal-only template through verbatim', async () => { + await expect(resolveTemplate('just text', scope())).resolves.toBe('just text'); + }); + + it('resolves inputs / ctx / run.outputs heads between literals', async () => { + const s = scope({ + inputs: { name: 'Ada' }, + ctx: { greeting: 'Hi' }, + outputs: { scan: { score: 9 } }, + }); + await expect(resolveTemplate('{{ctx.greeting}}, {{inputs.name}}!', s)).resolves.toBe( + 'Hi, Ada!', + ); + await expect(resolveTemplate('score={{run.outputs["scan"].score}}', s)).resolves.toBe( + 'score=9', + ); + }); + + it('stringifies number and boolean values', async () => { + const s = scope({ inputs: { n: 42, flag: true } }); + await expect(resolveTemplate('{{inputs.n}}/{{inputs.flag}}', s)).resolves.toBe('42/true'); + }); + + it('applies the json filter as 2-space pretty JSON', async () => { + const s = scope({ outputs: { scan: { a: 1 } } }); + await expect(resolveTemplate('{{run.outputs["scan"] | json}}', s)).resolves.toBe( + JSON.stringify({ a: 1 }, null, 2), + ); + }); + + it('applies length to a string, an array, and an object', async () => { + const s = scope({ inputs: { str: 'abcd', arr: [1, 2, 3], obj: { a: 1, b: 2 } } }); + await expect(resolveTemplate('{{inputs.str | length}}', s)).resolves.toBe('4'); + await expect(resolveTemplate('{{inputs.arr | length}}', s)).resolves.toBe('3'); + await expect(resolveTemplate('{{inputs.obj | length}}', s)).resolves.toBe('2'); + }); + + it('uses default only when the value is null/undefined, else passes the value through', async () => { + const s = scope({ inputs: { present: 'kept' }, outputs: {} }); + await expect( + resolveTemplate('{{run.outputs["missing"] | default("fallback")}}', s), + ).resolves.toBe('fallback'); + await expect(resolveTemplate('{{inputs.present | default("fallback")}}', s)).resolves.toBe( + 'kept', + ); + }); + + it('default keeps a falsy-but-present value (0 / false / empty string), rescues only missing', async () => { + await expect( + resolveTemplate('{{inputs.z | default("FB")}}', scope({ inputs: { z: 0 } })), + ).resolves.toBe('0'); + await expect( + resolveTemplate('{{inputs.f | default("FB")}}', scope({ inputs: { f: false } })), + ).resolves.toBe('false'); + await expect( + resolveTemplate('{{inputs.e | default("FB")}}', scope({ inputs: { e: '' } })), + ).resolves.toBe(''); + await expect( + resolveTemplate('{{run.outputs["m"] | default("FB")}}', scope({ outputs: {} })), + ).resolves.toBe('FB'); + }); + + it('chains filters left to right (default rescues, then length counts)', async () => { + const s = scope({ outputs: {} }); + await expect( + resolveTemplate('{{run.outputs["x"] | default("abc") | length}}', s), + ).resolves.toBe('3'); + }); + + it('reads a file through an injected sync or async capability', async () => { + const s = scope({ inputs: { path: 'src/a.ts' } }); + await expect( + resolveTemplate('{{inputs.path | read_file}}', s, { readFile: (p) => `SYNC:${p}` }), + ).resolves.toBe('SYNC:src/a.ts'); + await expect( + resolveTemplate('{{inputs.path | read_file}}', s, { + readFile: (p) => Promise.resolve(`ASYNC:${p}`), + }), + ).resolves.toBe('ASYNC:src/a.ts'); + }); +}); + +describe('resolveTemplate — typed, secret-free errors', () => { + it('unresolved_reference when a head/path yields nothing and no default rescues it', async () => { + await expectCode('{{inputs.missing}}', scope(), 'unresolved_reference'); + }); + + it('unknown_namespace for a non inputs/ctx/run.outputs head (incl. secrets)', async () => { + await expectCode('{{foo.bar}}', scope(), 'unknown_namespace'); + const secretErr = await expectCode('{{secrets.token}}', scope(), 'unknown_namespace'); + expect(secretErr.message).toContain('secret'); // a clearer message than the generic unknown case + }); + + it('treats a prototype key on a scope bag as a missing reference (no inherited member)', async () => { + // A prototype key on any of the three namespaces must not return an inherited member. + await expectCode('{{inputs.toString}}', scope({ inputs: {} }), 'unresolved_reference'); + await expectCode('{{ctx.constructor}}', scope({ ctx: {} }), 'unresolved_reference'); + await expectCode('{{run.outputs["toString"]}}', scope({ outputs: {} }), 'unresolved_reference'); + }); + + it('aborts a single resolveTemplate between segments when the signal has fired', async () => { + const aborted = new AbortController(); + aborted.abort(); + await expectCode( + '{{inputs.a}}{{inputs.b}}', + scope({ inputs: { a: '1', b: '2' } }), + 'aborted', + undefined, + aborted.signal, + ); + }); + + it('unserializable when an object/array is used as text without a json filter', async () => { + const s = scope({ outputs: { scan: { a: 1 }, list: [1, 2] } }); + await expectCode('{{run.outputs["scan"]}}', s, 'unserializable'); + await expectCode('{{run.outputs["list"]}}', s, 'unserializable'); + }); + + it('json wraps a circular structure as a typed unserializable error (not a raw TypeError)', async () => { + const circular: Record = {}; + circular['self'] = circular; + const err = await expectCode( + '{{run.outputs["x"] | json}}', + scope({ outputs: { x: circular } }), + 'unserializable', + ); + expect(err.message).not.toContain('circular'); // the raw TypeError detail stays on cause + expect(err.cause).toBeInstanceOf(TypeError); + }); + + it('json wraps a BigInt as a typed unserializable error', async () => { + await expectCode( + '{{run.outputs["x"] | json}}', + scope({ outputs: { x: { n: 1n } } }), + 'unserializable', + ); + }); + + it('unknown_filter for a filter not in the registry', async () => { + await expectCode('{{inputs.x | nope}}', scope({ inputs: { x: 'v' } }), 'unknown_filter'); + }); + + it('unknown_filter for an inherited registry member used as a filter name (no prototype call)', async () => { + const s = scope({ inputs: { x: 'v' } }); + await expectCode('{{inputs.x | toString}}', s, 'unknown_filter'); + await expectCode('{{inputs.x | constructor}}', s, 'unknown_filter'); + await expectCode('{{inputs.x | __proto__}}', s, 'unknown_filter'); + }); + + it('filter_arity for the wrong number of arguments', async () => { + const s = scope({ inputs: { x: 'v' } }); + await expectCode('{{inputs.x | default}}', s, 'filter_arity'); // needs 1 + await expectCode('{{inputs.x | json(1)}}', s, 'filter_arity'); // needs 0 + }); + + it('filter_type when a filter cannot apply to the value', async () => { + await expectCode('{{inputs.n | length}}', scope({ inputs: { n: 5 } }), 'filter_type'); + await expectCode('{{inputs.u | json}}', scope({ inputs: { u: undefined } }), 'filter_type'); + }); + + it('invalid_path for a malformed property access after the head', async () => { + const s = scope({ outputs: { x: { score: 1 } } }); + await expectCode('{{run.outputs["x"]..score}}', s, 'invalid_path'); + }); + + it('read_file_unavailable when no host reader was provided', async () => { + await expectCode( + '{{inputs.p | read_file}}', + scope({ inputs: { p: 'a.ts' } }), + 'read_file_unavailable', + ); + }); + + it('read_file_failed (keeping the host error on cause) when the reader throws', async () => { + const boom = new Error('ENOENT: /abs/secret/path'); + const err = await expectCode( + '{{inputs.p | read_file}}', + scope({ inputs: { p: 'a.ts' } }), + 'read_file_failed', + { + readFile: () => { + throw boom; + }, + }, + ); + expect(err.message).not.toContain('/abs/secret/path'); // the absolute path stays off the message + expect(err.cause).toBe(boom); // …but is preserved on cause for logs + }); + + it('read_file filter_type when the piped value is not a string path', async () => { + await expectCode('{{inputs.n | read_file}}', scope({ inputs: { n: 7 } }), 'filter_type', { + readFile: (p) => p, + }); + }); + + it('carries the offending {{ … }} verbatim as the error location', async () => { + const err = await expectCode('a {{inputs.missing}} b', scope(), 'unresolved_reference'); + expect(err.location).toBe('{{inputs.missing}}'); + }); +}); + +describe('resolveContext — eager-once, frozen, deterministic', () => { + const WF = `schema_version: '1.0' +workflow: + id: w + inputs: + - name: p + type: string + context: + - key: a + value: 'hello {{inputs.p}}' + - key: b + value: '{{ctx.a}}!' + nodes: + - id: n + type: input + edges: []`; + + it('resolves entries in order so a later entry can read an earlier one', async () => { + const ctx = await resolveContext(parseWorkflow(WF), { p: 'world' }); + expect(ctx).toEqual({ a: 'hello world', b: 'hello world!' }); + }); + + it('freezes the snapshot and re-resolves to an identical scope', async () => { + const wf = parseWorkflow(WF); + const first = await resolveContext(wf, { p: 'world' }); + const second = await resolveContext(wf, { p: 'world' }); + expect(Object.isFrozen(first)).toBe(true); + expect(second).toEqual(first); + }); + + it('resolves a read_file context entry through the injected capability', async () => { + const wf = parseWorkflow(`schema_version: '1.0' +workflow: + id: w + inputs: + - name: path + type: file_path + context: + - key: code + value: '{{inputs.path | read_file}}' + nodes: + - id: n + type: input + edges: []`); + const ctx = await resolveContext(wf, { path: 'x.ts' }, { readFile: (p) => `FILE(${p})` }); + expect(ctx).toEqual({ code: 'FILE(x.ts)' }); + }); + + it('rejects (pre-run) when a context pipe-filter fails, keeping the host path off the message', async () => { + const wf = parseWorkflow(`schema_version: '1.0' +workflow: + id: w + inputs: + - name: path + type: file_path + context: + - key: code + value: '{{inputs.path | read_file}}' + nodes: + - id: n + type: input + edges: []`); + const boom = new Error('ENOENT: /abs/missing.ts'); + let thrown: unknown; + try { + await resolveContext( + wf, + { path: 'missing.ts' }, + { + readFile: () => { + throw boom; + }, + }, + ); + } catch (err) { + thrown = err; + } + expect(thrown).toBeInstanceOf(InterpolationError); + if (!(thrown instanceof InterpolationError)) { + throw new Error('expected an InterpolationError'); + } + expect(thrown.code).toBe('read_file_failed'); + expect(thrown.message).not.toContain('/abs/missing.ts'); + expect(thrown.cause).toBe(boom); + }); + + it('returns a frozen empty snapshot when there is no context', async () => { + const wf = parseWorkflow(`schema_version: '1.0' +workflow: + id: w + nodes: + - id: n + type: input + edges: []`); + const ctx = await resolveContext(wf, {}); + expect(ctx).toEqual({}); + expect(Object.isFrozen(ctx)).toBe(true); + }); + + it('stores a `__proto__` context key as a real own property (null-prototype accumulator)', async () => { + const wf = parseWorkflow(`schema_version: '1.0' +workflow: + id: w + context: + - key: __proto__ + value: 'safe' + nodes: + - id: n + type: input + edges: []`); + const ctx = await resolveContext(wf, {}); + expect(Object.hasOwn(ctx, '__proto__')).toBe(true); + expect(Reflect.get(ctx, '__proto__')).toBe('safe'); // read via Reflect (the literal accessor is deprecated) + }); + + it('a backward context reference is unresolved at runtime (single-pass declared order)', async () => { + // `early` reads `late`, declared after it — accepted at parse, unresolved at runtime. + const wf = parseWorkflow(`schema_version: '1.0' +workflow: + id: w + context: + - key: early + value: '{{ctx.late}}' + - key: late + value: 'L' + nodes: + - id: n + type: input + edges: []`); + await expect(resolveContext(wf, {})).rejects.toBeInstanceOf(InterpolationError); + + // … unless a default rescues it. + const wf2 = parseWorkflow(`schema_version: '1.0' +workflow: + id: w + context: + - key: early + value: '{{ctx.late | default("FB")}}' + - key: late + value: 'L' + nodes: + - id: n + type: input + edges: []`); + const ctx = await resolveContext(wf2, {}); + expect(ctx['early']).toBe('FB'); + }); + + it('forwards the AbortSignal to the host readFile and aborts an already-cancelled run', async () => { + const wf = parseWorkflow(`schema_version: '1.0' +workflow: + id: w + inputs: + - name: path + type: file_path + context: + - key: code + value: '{{inputs.path | read_file}}' + nodes: + - id: n + type: input + edges: []`); + let receivedSignal: unknown; + const live = new AbortController(); + await resolveContext( + wf, + { path: 'x.ts' }, + { + readFile: (path, signal) => { + receivedSignal = signal; + return `FILE(${path})`; + }, + }, + live.signal, + ); + expect(receivedSignal).toBe(live.signal); + + const aborted = new AbortController(); + aborted.abort(); + // The rejection must be the cancellation discriminant, not a generic read failure. + await expect( + resolveContext(wf, { path: 'x.ts' }, { readFile: (p) => `FILE(${p})` }, aborted.signal), + ).rejects.toMatchObject({ code: 'aborted' }); + }); + + it('re-resolves a read_file context identically (determinism across the impurity seam)', async () => { + const wf = parseWorkflow(`schema_version: '1.0' +workflow: + id: w + inputs: + - name: path + type: file_path + context: + - key: code + value: '{{inputs.path | read_file}}' + nodes: + - id: n + type: input + edges: []`); + const reader = (path: string): string => `FILE(${path})`; + const first = await resolveContext(wf, { path: 'x.ts' }, { readFile: reader }); + const second = await resolveContext(wf, { path: 'x.ts' }, { readFile: reader }); + expect(second).toEqual(first); + }); +}); diff --git a/packages/core/src/interpolation/resolve.ts b/packages/core/src/interpolation/resolve.ts new file mode 100644 index 00000000..b9c99e92 --- /dev/null +++ b/packages/core/src/interpolation/resolve.ts @@ -0,0 +1,165 @@ +/** + * The `{{ … }}` runtime resolver (1.L2) — turn an authored template into concrete text by evaluating + * each reference against a {@link RunScope} and applying its pipe filters in order. Every node's input + * flows through here (workflow-yaml-spec.md §Context-and-interpolation). + * + * Pure but for the host-injected `readFile` capability: given the same scope and capabilities, a + * template resolves identically every time — the determinism the checkpoint/resume model (1.R) needs. + * `resolveContext` realizes the spec's **eager-once, immutable** context: every `context` entry is + * resolved a single time, in declared order (so a later entry may read an earlier one), into a frozen + * snapshot. Resolution never mutates the scope and never reaches the filesystem directly. + * + * What this module does NOT do: it never resolves a `secret` into agent/human text — the parse-time + * taint gate (`analyzeSecretTaint`) already rejected that, and {@link RunScope} carries no `secrets` + * namespace. The whole-string JS fields (`condition`/`transform`/`merge_fn`) are not templates; they + * belong to the expression sandbox (1.AB), not here. + * + * Provenance: `resolveTemplate` returns a plain string, flattening any `read_file`- / `run.outputs`- + * derived (untrusted) content together with literals — it carries no taint marker. The structural + * "untrusted-content-as-data" guarantee (docs/standards/security-review.md) binds the message-assembly + * layer (1.O/1.T/1.V): a resolved field that drew on an untrusted source must be placed only in a + * `user`/`tool` position, never `system`. That re-tainting is a 1.O/1.O-run-loop acceptance criterion, + * not something this pure resolver can enforce once provenance is flattened. + */ + +import type { AbortSignalLike, Workflow } from '@relavium/shared'; + +import { InterpolationError } from '../errors.js'; + +import { filterFn } from './filters.js'; +import { getByPath } from './path.js'; +import { parseTemplate, type InterpolationReference } from './references.js'; +import type { ResolverCapabilities, RunScope } from './scope.js'; + +/** + * Resolve an authored template string to concrete text. A literal segment passes through verbatim; a + * `{{ … }}` reference is evaluated against `scope` and stringified. + * @throws {InterpolationError} on an unknown namespace/filter, a bad filter application, an + * unserializable object used as text, or a reference that resolves to nothing without a `default`. + */ +export async function resolveTemplate( + text: string, + scope: RunScope, + caps: ResolverCapabilities = {}, + signal?: AbortSignalLike, +): Promise { + let out = ''; + for (const segment of parseTemplate(text)) { + if (segment.kind === 'literal') { + out += segment.text; + } else { + abortIfCancelled(signal); + const value = await resolveReference(segment.reference, scope, caps, signal); + out += stringify(value, segment.reference); + } + } + return out; +} + +/** + * Eagerly resolve every `context` entry exactly once into a frozen, immutable snapshot (the spec's + * eager-once context). Entries resolve in declared order, each seeing the inputs and the already- + * resolved context; the result is `Object.freeze`d so the run scope cannot drift mid-run. + * @throws {InterpolationError} when a context value cannot be resolved (e.g. `read_file` on a bad path). + */ +export async function resolveContext( + workflow: Workflow, + inputs: Readonly>, + caps: ResolverCapabilities = {}, + signal?: AbortSignalLike, +): Promise>> { + // A null-prototype accumulator so a context key named `__proto__`/`constructor` is stored as a real + // own property rather than being silently dropped (or mutating a prototype). NOTE for 1.R: this + // null-proto guard is in-memory only — when this frozen `ctx` is persisted/transported for + // checkpoint/resume it MUST go through `structuredClone` (which preserves the null prototype), never + // `JSON.stringify` → `JSON.parse`, which would re-materialize a `__proto__` key as a real setter. + const ctx = Object.create(null) as Record; + for (const entry of workflow.workflow.context ?? []) { + abortIfCancelled(signal); + // No node has run yet, so `outputs` is empty; a `{{run.outputs[…]}}` reference here is already + // rejected at parse (`analyzePreRunReferences`), so this only ever serves `inputs`/`ctx`. + const scope: RunScope = { inputs, ctx, outputs: {} }; + ctx[entry.key] = await resolveTemplate(entry.value, scope, caps, signal); + } + return Object.freeze(ctx); +} + +/** Resolve a single reference: head → trailing path → pipe filters (in order). */ +async function resolveReference( + ref: InterpolationReference, + scope: RunScope, + caps: ResolverCapabilities, + signal?: AbortSignalLike, +): Promise { + let value = getByPath(resolveHead(ref, scope), ref.path, ref.raw); + for (const filter of ref.filters) { + value = await filterFn(filter, ref)(value, filter.args, caps, ref, signal); + } + return value; +} + +/** Read the reference head from the run scope; an unserved namespace is a typed error. */ +function resolveHead(ref: InterpolationReference, scope: RunScope): unknown { + switch (ref.kind) { + case 'inputs': + return ownValue(scope.inputs, ref.identifier); + case 'ctx': + return ownValue(scope.ctx, ref.identifier); + case 'node': + return ownValue(scope.outputs, ref.identifier); + case 'secrets': + throw new InterpolationError( + 'unknown_namespace', + `\`secrets.*\` is not a runtime namespace — a \`secret\`-typed input feeds credential fields, never resolved text`, + { location: ref.raw }, + ); + case 'unknown': + throw new InterpolationError( + 'unknown_namespace', + `cannot resolve \`${ref.identifier}\` — not an inputs/ctx/run.outputs reference`, + { location: ref.raw }, + ); + } +} + +/** + * Read an OWN property only. A scope bag is assembled by the host from external data, so a + * prototype/polluted key (`toString`, `__proto__`, `constructor`) is treated as a missing reference + * (→ `undefined`, then `default`/`unresolved_reference`) rather than returning an inherited member. + */ +function ownValue(bag: Readonly>, key: string): unknown { + return Object.hasOwn(bag, key) ? bag[key] : undefined; +} + +/** + * Cooperative cancellation between resolution steps: throw a typed `aborted` error once the run's + * signal has fired. (`AbortSignalLike` is the engine's DOM/node-free signal type, so it exposes + * `aborted` rather than `throwIfAborted()`; the same signal also forwards to the host `readFile`.) + */ +function abortIfCancelled(signal: AbortSignalLike | undefined): void { + if (signal?.aborted === true) { + throw new InterpolationError('aborted', 'interpolation was aborted'); + } +} + +/** Turn a resolved value into text — primitives stringify; an object needs an explicit `| json`. */ +function stringify(value: unknown, ref: InterpolationReference): string { + if (value === undefined || value === null) { + throw new InterpolationError( + 'unresolved_reference', + `\`${ref.raw}\` resolved to no value — check the reference or add a \`| default(…)\` filter`, + { location: ref.raw }, + ); + } + if (typeof value === 'string') { + return value; + } + if (typeof value === 'number' || typeof value === 'boolean' || typeof value === 'bigint') { + return String(value); + } + throw new InterpolationError( + 'unserializable', + `\`${ref.raw}\` resolved to ${Array.isArray(value) ? 'a list' : 'an object'} — add a \`| json\` filter to embed it as text`, + { location: ref.raw }, + ); +} diff --git a/packages/core/src/interpolation/scope.ts b/packages/core/src/interpolation/scope.ts new file mode 100644 index 00000000..c0820c9f --- /dev/null +++ b/packages/core/src/interpolation/scope.ts @@ -0,0 +1,49 @@ +/** + * The run scope the interpolation resolver reads from, and the host capabilities it may call (1.L2). + * + * `RunScope` is plain, immutable data the engine assembles as a run progresses: 1.M builds the + * per-node inputs, 1.O fills `outputs` as nodes complete, and `resolveContext` (this module's sibling) + * produces the frozen `ctx` snapshot. The resolver only reads it — it never mutates the scope. + * + * `ResolverCapabilities` is the **purity seam**. The engine has zero platform-specific imports + * (CLAUDE.md rule 5), so any filter that needs I/O — today only `read_file` — calls a host-supplied + * function instead of reaching for `node:fs`. The host (CLI / desktop / VS Code) owns the capability + * and its workspace-root sandbox; the engine passes the authored argument through verbatim. A + * capability is optional: a template that needs an absent one fails with a typed `InterpolationError`, + * never a crash, and never a platform import sneaking into `packages/core`. + * + * There is deliberately **no `secrets` namespace** here. The v1.0 authored surface + * (workflow-yaml-spec.md §Context-and-interpolation) exposes `inputs`, `ctx`, and `run.outputs`; a + * `secret`-typed input lives in `inputs` like any other value. The parse-time taint gate + * (`analyzeSecretTaint`) has already rejected every secret reference from agent/human text before a + * scope is ever resolved, so a secret value never flows through the text path by construction. + */ + +import type { AbortSignalLike } from '@relavium/shared'; + +/** The run-scope namespaces a `{{ … }}` reference may read against. */ +export interface RunScope { + /** Declared workflow inputs, resolved to their values — `{{inputs.}}`. */ + readonly inputs: Readonly>; + /** The eagerly-resolved, frozen context snapshot — `{{ctx.}}`. */ + readonly ctx: Readonly>; + /** Completed node outputs, keyed by node id — `{{run.outputs[""]}}`. */ + readonly outputs: Readonly>; +} + +/** Side-effecting capabilities the host injects because the pure engine cannot implement them. */ +export interface ResolverCapabilities { + /** + * Read a workspace file's text for the `read_file` filter. The engine passes the authored path + * through **unchanged** and never touches the filesystem itself, so the host reader **must jail to + * the workspace root and reject path traversal** — that sandbox duty is delegated, not optional, and + * is a mandatory-review trigger (docs/standards/security-review.md §When a review is mandatory). The + * `path` argument is whatever the template resolved to and may carry a value the engine cannot prove + * non-secret (e.g. `{{inputs.x | read_file}}`), so the host must not log it. The optional `signal` + * lets a cancelled run abort a slow or hung read. The reader must be a stable read-once snapshot for + * a given path within a run, so a re-resolve on resume is byte-identical (the 1.R determinism + * contract). May resolve synchronously or asynchronously. When absent, a `read_file` filter fails + * with a typed `InterpolationError` (`read_file_unavailable`), never a crash. + */ + readonly readFile?: (path: string, signal?: AbortSignalLike) => string | Promise; +} diff --git a/packages/core/src/parser.test.ts b/packages/core/src/parser.test.ts index e78e7fc1..aa10a72f 100644 --- a/packages/core/src/parser.test.ts +++ b/packages/core/src/parser.test.ts @@ -1,7 +1,7 @@ import { stringify as stringifyYaml } from 'yaml'; import { describe, expect, it } from 'vitest'; -import { WorkflowSyntaxError, WorkflowValidationError } from './errors.js'; +import { WorkflowSecretLeakError, WorkflowSyntaxError, WorkflowValidationError } from './errors.js'; import { collectReferences } from './interpolation/collect.js'; import { parseWorkflow } from './parser.js'; @@ -263,15 +263,26 @@ describe('parseWorkflow — malformed (each fails with a field-named, secret-fre expect(err.issues[0]?.message).toMatch(/kebab/); }); + it('does not echo a kebab-invalid (underscore) node id — SAFE_ID_LABEL mirrors kebabIdSchema', () => { + // A lowercase id with underscores fails the hyphen-only kebab schema; the locator must NOT echo it + // (it would otherwise reflect a secret-shaped `sk_live_…` value), falling back to `node #0`. + const secret = 'sk_live_do_not_echo'; + const err = expectValidationError( + doc(` id: w\n nodes:\n - id: ${secret}\n type: input\n edges: []`), + ); + expect(JSON.stringify(err.issues)).not.toContain(secret); + expect(err.issues[0]?.field).toBe('node #0.id'); + }); + it('surfaces a structural message for the `too_small` code path (min-1 string constraint)', () => { - // An empty string on context[].key (nonEmptyString = z.string().min(1)) → Zod code `too_small`. + // An empty agent `system_prompt` (nonEmptyString = z.string().min(1)) → Zod code `too_small`. // messageFor returns issue.message directly — pin that it names the constraint, not the authored value. const err = expectValidationError( doc( - ` id: w\n context:\n - key: ''\n value: v\n nodes:\n - id: n\n type: input\n edges: []`, + ` id: w\n agents:\n - id: ag\n name: A\n model: claude-sonnet-4-6\n provider: anthropic\n system_prompt: ''\n nodes:\n - id: n\n type: input\n edges: []`, ), ); - const issue = err.issues.find((i) => i.field.includes('context')); + const issue = err.issues.find((i) => i.field.includes('system_prompt')); expect(issue).toBeDefined(); expect(issue?.message).toMatch(/character|length|least/i); }); @@ -348,6 +359,17 @@ describe('parseWorkflow — diagnostic field naming (issue-mapper coverage)', () expect(err.issues[0]?.message).toMatch(/expected one of:/); }); + it('still names an input whose (now schema-legal) name is uppercase/underscore (SAFE_NAME_LABEL)', () => { + // `API_KEY` passes `interpolationNameSchema` but not the kebab id charset — the locator must use the + // name charset, not degrade to a positional `#0`. + const err = expectValidationError( + doc( + ` id: w\n inputs:\n - name: API_KEY\n nodes:\n - id: n\n type: input\n edges: []`, + ), + ); + expect(err.issues[0]?.field).toBe('input `API_KEY`.type'); // missing `type` → named, not `input #0` + }); + it('falls back to an index when a context entry is missing its key', () => { const err = expectValidationError( doc( @@ -421,19 +443,64 @@ describe('parseWorkflow — diagnostic field naming (issue-mapper coverage)', () }); }); -describe('collectReferences — context/run.outputs (1.M known gap)', () => { - it('permits a context value that references run.outputs — enforcement deferred to the DAG builder (1.M)', () => { - // TODO(1.M): workflow-yaml-spec.md forbids {{run.outputs[...]}} inside context[].value because - // context is resolved pre-run. Once the DAG builder rejects unsatisfiable node-output edges in a - // context site, replace this test with a WorkflowValidationError assertion from parseWorkflow. - const wf = parseWorkflow( +describe('parseWorkflow — context referencing run.outputs (1.L2 static gate)', () => { + it('rejects a context value that references run.outputs (resolved before any node runs)', () => { + // workflow-yaml-spec.md §Context-and-interpolation: context is eagerly resolved pre-run, so a + // node output is unavailable — `analyzePreRunReferences` makes this a field-named parse error. + const err = expectValidationError( `schema_version: '1.0'\nworkflow:\n id: w\n context:\n - key: snapshot\n value: '{{run.outputs["some-node"]}}'\n nodes:\n - id: some-node\n type: input\n edges: []`, ); - const sites = collectReferences(wf); - const ctxSite = sites.find((s) => s.location === 'context `snapshot`.value'); - // The reference is CLASSIFIED (kind:'node') but not VALIDATED — the gap is intentional for now. - expect(ctxSite?.references[0]?.kind).toBe('node'); - expect(ctxSite?.references[0]?.identifier).toBe('some-node'); + expect(err.issues[0]?.field).toBe('context `snapshot`.value'); + expect(err.issues[0]?.message).toContain('run.outputs'); + }); + + it('permits a context value that references inputs/ctx (the legitimate pre-run sources)', () => { + const wf = parseWorkflow( + `schema_version: '1.0'\nworkflow:\n id: w\n inputs:\n - name: p\n type: string\n context:\n - key: snapshot\n value: 'for {{inputs.p}}'\n nodes:\n - id: n\n type: input\n edges: []`, + ); + const ctxSite = collectReferences(wf).find((s) => s.location === 'context `snapshot`.value'); + expect(ctxSite?.category).toBe('context-value'); + expect(ctxSite?.references[0]).toMatchObject({ kind: 'inputs', identifier: 'p' }); + }); +}); + +describe('parseWorkflow — secret interpolation (ADR-0029(c) static gate)', () => { + const LEAK = `schema_version: '1.0' +workflow: + id: w + inputs: + - name: api_key + type: secret + agents: + - id: ag + name: Ag + model: claude-sonnet-4-6 + provider: anthropic + system_prompt: 'system' + nodes: + - id: n + type: agent + agent_ref: ag + prompt_template: 'use {{inputs.api_key}}' + edges: []`; + + it('rejects at parse with a WorkflowSecretLeakError naming the field and the secret', () => { + let thrown: unknown; + try { + parseWorkflow(LEAK, { source: 'leak.yaml' }); + } catch (err) { + thrown = err; + } + expect(thrown).toBeInstanceOf(WorkflowSecretLeakError); + if (!(thrown instanceof WorkflowSecretLeakError)) { + throw new Error('expected a WorkflowSecretLeakError'); + } + expect(thrown.leaks[0]).toEqual({ + location: 'node `n`.prompt_template', + secret: 'inputs.api_key', + }); + expect(thrown.source).toBe('leak.yaml'); // the workspace-relative label is propagated + expect(thrown.message).toContain('`inputs.api_key`'); // names the symbol, never a resolved value }); }); @@ -463,9 +530,12 @@ function expectValidationError( let thrown: unknown; try { parseWorkflow(yamlText, opts); - } catch (caught) { - thrown = caught; + } catch (err) { + thrown = err; } expect(thrown).toBeInstanceOf(WorkflowValidationError); - return thrown as WorkflowValidationError; + if (!(thrown instanceof WorkflowValidationError)) { + throw new Error('expected a WorkflowValidationError'); + } + return thrown; } diff --git a/packages/core/src/parser.ts b/packages/core/src/parser.ts index f16d1af6..0cc6f0d1 100644 --- a/packages/core/src/parser.ts +++ b/packages/core/src/parser.ts @@ -1,13 +1,20 @@ /** - * `WorkflowYAMLParser` (1.L) — the engine's entry point. Loads a `.relavium.yaml` **string** and - * validates it against the strict `@relavium/shared` `WorkflowSchema` (ADR-0023), producing a typed - * `WorkflowDefinition` or a typed, field-named, secret-free error. + * `WorkflowYAMLParser` (1.L / 1.L2) — the engine's entry point. Loads a `.relavium.yaml` **string**, + * validates it against the strict `@relavium/shared` `WorkflowSchema` (ADR-0023), and runs the static + * interpolation gates (1.L2), producing a typed `WorkflowDefinition` or a typed, field-named, + * secret-free error. + * + * Three reject stages, in order: a YAML syntax fault → {@link WorkflowSyntaxError}; a schema failure + * or a context value that reads a node output → {@link WorkflowValidationError}; a secret reaching + * agent/human text → {@link WorkflowSecretLeakError} (ADR-0029(c)). All three are field-named and + * secret-free, so an invalid file never yields a `WorkflowDefinition` and a run never starts on one. * * Pure by contract: it takes text (never a path), reads no filesystem, touches no environment, and * holds no state — the host surface (CLI / desktop / VS Code) reads the file and passes the string * plus an optional workspace-relative label in. Node-existence, `$ref`/`agent_ref` resolution, handle - * resolution, and the cycle check are the DAG builder's job (1.M) on the returned object; interpolation - * EVALUATION and secret-taint are the resolver's job (1.L2). 1.L is shape-only. + * resolution, and the cycle check are the DAG builder's job (1.M); interpolation *evaluation* is the + * runtime resolver's job (`resolveTemplate`/`resolveContext`, 1.L2). The taint check here is static — + * it reads an input's *type*, never its value. */ import { LineCounter, parse as parseYaml, YAMLParseError } from 'yaml'; @@ -15,7 +22,13 @@ import type { ZodIssue } from 'zod'; import { WorkflowSchema, type Workflow } from '@relavium/shared'; -import { WorkflowSyntaxError, WorkflowValidationError, type WorkflowIssue } from './errors.js'; +import { + WorkflowSecretLeakError, + WorkflowSyntaxError, + WorkflowValidationError, + type WorkflowIssue, +} from './errors.js'; +import { analyzePreRunReferences, analyzeSecretTaint } from './interpolation/analyze.js'; /** The validated workflow document — `@relavium/shared`'s `Workflow`, under a parser-local alias. */ export type WorkflowDefinition = Workflow; @@ -74,7 +87,18 @@ export function parseWorkflow(yamlText: string, opts?: ParseWorkflowOptions): Wo // and `cause` is publicly reachable — the curated, secret-free `issues` are the diagnostic surface. throw new WorkflowValidationError(issues, source === undefined ? undefined : { source }); } - return result.data; + const definition = result.data; + + // Static interpolation gates (1.L2) over the now-typed definition — both read structure only. + const preRunIssues = analyzePreRunReferences(definition); + if (preRunIssues.length > 0) { + throw new WorkflowValidationError(preRunIssues, source === undefined ? undefined : { source }); + } + const leaks = analyzeSecretTaint(definition); + if (leaks.length > 0) { + throw new WorkflowSecretLeakError(leaks, source === undefined ? undefined : { source }); + } + return definition; } /** Normalize ANY parse-stage throw (a YAML fault, an anchor/alias `ReferenceError`, …) to a typed error. */ @@ -141,11 +165,17 @@ function locate(path: ReadonlyArray, root: unknown): string { } /** - * A well-formed identifier (the shape of a valid id/name/key) — only such a value is echoed into a - * field locator, so an INVALID authored value (e.g. an id that failed kebab validation, which may be - * arbitrary text or a misplaced secret) is never reflected back; it falls back to a positional `#n`. + * The shape of a valid authored identifier — only such a value is echoed into a field locator, so an + * INVALID authored value (which may be arbitrary text or a misplaced secret) is never reflected back; + * it falls back to a positional `#n`. Two charsets, each mirroring exactly what its field's schema + * permits: `SAFE_ID_LABEL` for `node`/`agent` ids (hyphen-only kebab, mirrors `@relavium/shared`'s + * `kebabIdSchema` — so a kebab-invalid id like `sk_live_x` is NOT echoed), and `SAFE_NAME_LABEL` for an + * `input` name / `context` key (the interpolation head charset, mirrors `interpolationNameSchema`). + * Keeping each aligned with its schema means a value the schema accepts (e.g. `API_KEY`) still names + * its own error, while one it rejects falls back to `#n`. */ -const SAFE_LABEL = /^[a-z0-9]+(?:[-_][a-z0-9]+)*$/; +const SAFE_ID_LABEL = /^[a-z0-9]+(?:-[a-z0-9]+)*$/; +const SAFE_NAME_LABEL = /^[A-Za-z0-9_-]+$/; function itemLabel( spec: Record | undefined, @@ -153,21 +183,21 @@ function itemLabel( index: number, ): string { const item = asRecord(asArray(spec?.[collection])?.[index]); - const named = (key: string, prefix: string): string => { + const named = (key: string, prefix: string, pattern: RegExp): string => { const value = item?.[key]; - return typeof value === 'string' && value.length <= 64 && SAFE_LABEL.test(value) + return typeof value === 'string' && value.length <= 64 && pattern.test(value) ? `${prefix} \`${value}\`` : `${prefix} #${index}`; }; switch (collection) { case 'nodes': - return named('id', 'node'); + return named('id', 'node', SAFE_ID_LABEL); case 'agents': - return named('id', 'agent'); + return named('id', 'agent', SAFE_ID_LABEL); case 'inputs': - return named('name', 'input'); + return named('name', 'input', SAFE_NAME_LABEL); case 'context': - return named('key', 'context'); + return named('key', 'context', SAFE_NAME_LABEL); case 'edges': return `edge #${index}`; default: diff --git a/packages/shared/src/common.ts b/packages/shared/src/common.ts index 8ebeb95f..14e0f3bf 100644 --- a/packages/shared/src/common.ts +++ b/packages/shared/src/common.ts @@ -24,6 +24,20 @@ export const kebabIdSchema = z /** A non-empty string. */ export const nonEmptyString = z.string().min(1); +/** + * An identifier referenceable from `{{inputs.}}` / `{{ctx.}}` (`workflow.inputs[].name`, + * `workflow.context[].key`). It must match the interpolation lexer's head charset (the `NAMESPACED` + * rule in `@relavium/core` `references.ts`) — otherwise a schema-valid name could never be referenced. + * Aligning the authored contract with the lexer is an ADR-0023 fail-loud tightening. + */ +export const INTERPOLATION_NAME_PATTERN = '[A-Za-z0-9_-]+'; +export const interpolationNameSchema = z + .string() + .regex( + new RegExp(`^${INTERPOLATION_NAME_PATTERN}$`), + 'must be referenceable in {{ … }} (letters, digits, `_` or `-`)', + ); + /** A positive integer (>= 1). */ export const positiveInt = z.number().int().positive(); diff --git a/packages/shared/src/workflow.test.ts b/packages/shared/src/workflow.test.ts index 065e82fb..716de936 100644 --- a/packages/shared/src/workflow.test.ts +++ b/packages/shared/src/workflow.test.ts @@ -270,6 +270,16 @@ describe('WorkflowSchema', () => { ).toBe(false); }); + it('rejects an input name / context key that is not referenceable in {{ … }}', () => { + // The lexer's head charset is [A-Za-z0-9_-]+, so a name with a space or dot could never be + // referenced — the schema must reject it (aligns the contract with the interpolation lexer). + expect(accepts(withWorkflow({ inputs: [{ name: 'my name', type: 'string' }] }))).toBe(false); + expect(accepts(withWorkflow({ inputs: [{ name: 'a.b', type: 'string' }] }))).toBe(false); + expect(accepts(withWorkflow({ context: [{ key: 'has space', value: 'v' }] }))).toBe(false); + // …but a normal snake/kebab identifier is accepted. + expect(accepts(withWorkflow({ inputs: [{ name: 'file_path', type: 'string' }] }))).toBe(true); + }); + it('rejects duplicate agent ids', () => { const agent = { id: 'dup', diff --git a/packages/shared/src/workflow.ts b/packages/shared/src/workflow.ts index 0b2b21e8..887fcf47 100644 --- a/packages/shared/src/workflow.ts +++ b/packages/shared/src/workflow.ts @@ -2,6 +2,7 @@ import { z } from 'zod'; import { findDuplicates, + interpolationNameSchema, kebabIdSchema, nonEmptyString, nonNegativeInt, @@ -110,7 +111,7 @@ const VALIDATION_KEYS_BY_TYPE: Record< export const WorkflowInputSchema = z .object({ - name: nonEmptyString, + name: interpolationNameSchema, // must be referenceable as `{{inputs.}}` type: InputTypeSchema, required: z.boolean().optional(), default: z.unknown().optional(), @@ -146,7 +147,7 @@ export type WorkflowInput = z.infer; /** A shared variable exposed as `{{ctx.key}}`. */ export const ContextEntrySchema = z .object({ - key: nonEmptyString, + key: interpolationNameSchema, // must be referenceable as `{{ctx.}}` value: z.string(), }) .strict();