Pre-launch — Gluecron is in final validation. Public signups and git hosting for non-owner users open after launch review.
ccantynz/Gluecron.com
Commit18b210dunknown_key

feat(workflow-runner): wire workflow secret substitution into v1 runner

feat(workflow-runner): wire workflow secret substitution into v1 runner

The workflow_secrets table (Block C1 / migration 0037), settings UI
(/:owner/:repo/settings/workflow-secrets), and CRUD lib have been on
disk since the workflow-engine-v2 sprint, but the runner never
actually substituted `\${{ secrets.NAME }}` into step.run — the
"v2 executor" in workflow-runner.ts that was supposed to call
substituteSecrets is gated off (Sprint 1 stub returning false), and
the v1 path passed step.run straight to bash. Result: every secret
showed up in logs as a literal token.

This adds the missing pure helper and plumbs it through the v1 path:

- src/lib/workflow-secrets.ts gains `substituteSecrets(template,
  secrets)` — pure regex replace of `\${{ <ws>* secrets <ws>* . <ws>*
  NAME <ws>* }}`. Strict name grammar (`[A-Z_][A-Z0-9_]*`); missing
  names left intact (loud failure signal); uses hasOwnProperty so
  {}.toString cannot be substituted.
- src/lib/workflow-runner.ts:
    * import substituteSecrets + loadSecretsContext.
    * runStep gains an optional `secrets` parameter (default {}).
      When step.run contains "\${{" the substitution runs; otherwise
      the hot path is unchanged. Existing callers stay green.
    * executeJob accepts an optional `secrets?` and forwards to
      runStep on every step.
    * executeRun's v1 fallback loads secrets once via
      loadSecretsContext(repositoryId), then passes them into each
      executeJob call. loadSecretsContext is fail-soft (empty map on
      missing master key, decrypt failures, or DB error) so this is
      safe for repos without secrets configured.

The v2 executor branch is still gated off; this commit only wires
the v1 path. When v2 is unblocked it can adopt the same `secrets`
arg without further plumbing.

Tests: +14 (substituteSecrets — single + multi tokens, whitespace
variants, repeated tokens, missing-name pass-through, strict name
grammar including lower/digit-leading rejection, defensive non-string
input, prototype-pollution guard, unrelated `\${{ env.X }}` /
`\${{ vars.X }}` left untouched). Total suite 1170 pass / 8 skip /
0 fail (was 1156 / 8 / 0).

https://claude.ai/code/session_0163vJChUuZqqtBBznUW6xBU
Claude committed on April 30, 2026Parent: 2afb411
3 files changed+189−318b210d6cbb8ba8118434548ba5f50c12bad4603
3 changed files+189−3
Addedsrc/__tests__/workflow-secrets-substitute.test.ts+113−0View fileUnifiedSplit
1/**
2 * Pure-helper tests for src/lib/workflow-secrets.ts substituteSecrets.
3 *
4 * loadSecretsContext / upsert / delete are DB-coupled so they're not
5 * exercised here — the crypto round-trip lives in
6 * workflow-secrets-crypto.test.ts. This file pins the pure substitution
7 * grammar so the runner's secret-injection contract can be relied on
8 * without instantiating Postgres.
9 */
10
11import { describe, it, expect } from "bun:test";
12import { substituteSecrets } from "../lib/workflow-secrets";
13
14describe("substituteSecrets — happy path", () => {
15 it("replaces a single token with the matching plaintext", () => {
16 const out = substituteSecrets(
17 'echo "$TOKEN_${{ secrets.TOKEN }}"',
18 { TOKEN: "abc123" }
19 );
20 expect(out).toBe('echo "$TOKEN_abc123"');
21 });
22
23 it("replaces multiple tokens in one template", () => {
24 const out = substituteSecrets(
25 "DEPLOY_KEY=${{ secrets.DEPLOY_KEY }} REGION=${{ secrets.REGION }}",
26 { DEPLOY_KEY: "k1", REGION: "us-east-1" }
27 );
28 expect(out).toBe("DEPLOY_KEY=k1 REGION=us-east-1");
29 });
30
31 it("tolerates whitespace variants inside the braces", () => {
32 const map = { X: "v" };
33 expect(substituteSecrets("${{secrets.X}}", map)).toBe("v");
34 expect(substituteSecrets("${{ secrets.X }}", map)).toBe("v");
35 expect(substituteSecrets("${{ secrets . X }}", map)).toBe("v");
36 });
37
38 it("repeats substitution when a name appears multiple times", () => {
39 const out = substituteSecrets(
40 "${{ secrets.X }} and again ${{ secrets.X }}",
41 { X: "yes" }
42 );
43 expect(out).toBe("yes and again yes");
44 });
45});
46
47describe("substituteSecrets — leaves tokens intact when secret is missing", () => {
48 it("missing name → token unchanged (loud failure signal)", () => {
49 const tpl = "echo ${{ secrets.MISSING }}";
50 expect(substituteSecrets(tpl, {})).toBe(tpl);
51 expect(substituteSecrets(tpl, { OTHER: "x" })).toBe(tpl);
52 });
53
54 it("substitutes the matching tokens and leaves the missing ones", () => {
55 const out = substituteSecrets(
56 "${{ secrets.A }} / ${{ secrets.B }} / ${{ secrets.C }}",
57 { A: "a", C: "c" }
58 );
59 expect(out).toBe("a / ${{ secrets.B }} / c");
60 });
61});
62
63describe("substituteSecrets — strict name grammar", () => {
64 it("rejects lowercase names (matches GitHub Actions grammar)", () => {
65 const tpl = "echo ${{ secrets.lower }}";
66 expect(substituteSecrets(tpl, { lower: "x" })).toBe(tpl);
67 });
68
69 it("rejects names starting with a digit", () => {
70 const tpl = "echo ${{ secrets.1ABC }}";
71 expect(substituteSecrets(tpl, { "1ABC": "x" })).toBe(tpl);
72 });
73
74 it("accepts underscore-only names + names with digits", () => {
75 expect(
76 substituteSecrets("${{ secrets.A_B_C }}", { A_B_C: "v" })
77 ).toBe("v");
78 expect(
79 substituteSecrets("${{ secrets._UNDER }}", { _UNDER: "v" })
80 ).toBe("v");
81 expect(
82 substituteSecrets("${{ secrets.X1Y2 }}", { X1Y2: "v" })
83 ).toBe("v");
84 });
85});
86
87describe("substituteSecrets — defensive on bad input", () => {
88 it("returns '' for non-string template", () => {
89 expect(substituteSecrets(undefined as any, {})).toBe("");
90 expect(substituteSecrets(null as any, {})).toBe("");
91 expect(substituteSecrets(42 as any, {})).toBe("");
92 });
93
94 it("returns the template untouched when secrets is null/undefined", () => {
95 expect(substituteSecrets("hello", null as any)).toBe("hello");
96 expect(substituteSecrets("hello", undefined as any)).toBe("hello");
97 });
98
99 it("returns '' when both inputs are empty", () => {
100 expect(substituteSecrets("", {})).toBe("");
101 });
102
103 it("ignores prototype-pollution probes (uses hasOwnProperty)", () => {
104 const tpl = "${{ secrets.TO_STRING }}";
105 // {}.toString exists on the prototype; substitution must NOT pick it up.
106 expect(substituteSecrets(tpl, {} as any)).toBe(tpl);
107 });
108
109 it("does not alter unrelated `${{ ... }}` syntax (env, vars)", () => {
110 const tpl = "${{ env.FOO }} ${{ vars.BAR }}";
111 expect(substituteSecrets(tpl, { FOO: "v" })).toBe(tpl);
112 });
113});
Modifiedsrc/lib/workflow-runner.ts+36−3View fileUnifiedSplit
2727 workflowRuns,
2828 workflows,
2929} from "../db/schema";
30import {
31 loadSecretsContext,
32 substituteSecrets,
33} from "./workflow-secrets";
3034
3135// ---------------------------------------------------------------------------
3236// Tunables
190194 * Run a single step via `bash -c`. Captures stdout/stderr (capped), enforces
191195 * a hard timeout with SIGTERM → SIGKILL escalation, and returns a StepResult
192196 * shaped for persistence.
197 *
198 * `secrets` is the per-run plaintext map produced by
199 * `loadSecretsContext(repoId)`. Default `{}` keeps the legacy callers
200 * working — when no secrets are loaded the substitution pass is a no-op
201 * because the regex matches nothing.
193202 */
194203async function runStep(
195204 step: ParsedStep,
196205 checkoutDir: string,
197 runId: string
206 runId: string,
207 secrets: Record<string, string> = {}
198208): Promise<StepResult> {
199209 const name =
200210 typeof step.name === "string" && step.name.length > 0
201211 ? step.name
202212 : (typeof step.run === "string" ? step.run.split("\n")[0] : "") ||
203213 "step";
204 const run = typeof step.run === "string" ? step.run : "";
214 const rawRun = typeof step.run === "string" ? step.run : "";
215 // Substitute `${{ secrets.NAME }}` only when the template actually
216 // contains a token — otherwise this is a free pass-through. The function
217 // is pure + safe for empty secret maps; the conditional avoids an
218 // unnecessary regex compile + scan on the hot path.
219 const run =
220 rawRun.indexOf("${{") >= 0
221 ? substituteSecrets(rawRun, secrets)
222 : rawRun;
205223 const started = Date.now();
206224
207225 if (!run) {
386404 job: ParsedJob;
387405 jobOrder: number;
388406 checkoutDir: string;
407 /** Per-run plaintext secrets. Default `{}` preserves the v1 contract
408 * for callers that haven't been updated to plumb secrets through. */
409 secrets?: Record<string, string>;
389410}): Promise<{ success: boolean }> {
390411 const { runId, jobKey, job, jobOrder, checkoutDir } = opts;
391412 const name = typeof job.name === "string" && job.name ? job.name : jobKey;
441462 });
442463 continue;
443464 }
444 const result = await runStep(step, checkoutDir, runId);
465 const result = await runStep(step, checkoutDir, runId, opts.secrets || {});
445466 stepResults.push(result);
446467 logParts.push(
447468 `==> ${result.name}\n$ ${result.run}\n${result.stdout}${
631652
632653 // --- Run jobs sequentially (v1 fallback) ---
633654 if (!handledByV2) {
655 // Load per-run secrets once. loadSecretsContext is fail-soft — empty
656 // map on missing master key, decrypt failures, or DB error — so this
657 // is always safe to call. A small overhead per run; secret-free
658 // workflows just see {} pass through.
659 let runSecrets: Record<string, string> = {};
660 try {
661 runSecrets = await loadSecretsContext(run.repositoryId);
662 } catch (err) {
663 console.error("[workflow-runner] loadSecretsContext threw:", err);
664 runSecrets = {};
665 }
634666 try {
635667 for (let i = 0; i < jobs.length; i++) {
636668 const { key, job } = jobs[i]!;
640672 job,
641673 jobOrder: i,
642674 checkoutDir,
675 secrets: runSecrets,
643676 });
644677 if (!result.success) {
645678 anyJobFailed = true;
Modifiedsrc/lib/workflow-secrets.ts+40−0View fileUnifiedSplit
182182 }
183183}
184184
185/**
186 * Substitute `${{ secrets.NAME }}` references inside a template string with
187 * the corresponding plaintext from `secrets`. Pure helper — no DB, no
188 * side effects. Designed to feed the workflow runner's per-step `run:`
189 * value before handing it to `bash -c`.
190 *
191 * Behaviour:
192 * - Whitespace inside the `{{ ... }}` is tolerated: `${{secrets.X}}`,
193 * `${{ secrets.X }}`, `${{ secrets . X }}`.
194 * - Names that don't appear in `secrets` are left intact (so the
195 * unsubstituted token shows up in logs and surfaces the misconfig
196 * loudly — matches the loadSecretsContext docstring contract).
197 * - Names that don't match the GitHub-Actions secret-name grammar
198 * (`[A-Z_][A-Z0-9_]*`) are also left intact, defence-in-depth
199 * against malformed YAML producing weird tokens.
200 * - The function never throws on bad input; non-string templates
201 * return `""`.
202 *
203 * Pure regex; no exec-on-string allocations beyond the single replace.
204 */
205export function substituteSecrets(
206 template: string,
207 secrets: Record<string, string>
208): string {
209 if (typeof template !== "string") return "";
210 if (!template || !secrets) return template || "";
211 // ${{ <ws>* secrets <ws>* . <ws>* NAME <ws>* }}
212 // We split the regex on the dot so we can tolerate whitespace either
213 // side of it; the NAME group is captured with the strict grammar so a
214 // malformed identifier won't accidentally substitute.
215 return template.replace(
216 /\$\{\{\s*secrets\s*\.\s*([A-Z_][A-Z0-9_]*)\s*\}\}/g,
217 (match, name: string) => {
218 if (Object.prototype.hasOwnProperty.call(secrets, name)) {
219 return secrets[name];
220 }
221 return match;
222 }
223 );
224}
185225/**
186226 * Build the `{ NAME: plaintext }` map consumed by the runner's
187227 * `substituteSecrets(template, secrets)` calls.
188228