Skip to content

[security] a flow's inbound-hook secret (config.secret on the start node) is served in cleartext by the flow-definition read; after #20529 every armed hook carries one #20552

Closed

Description

@objectstack-fleet

Filing gate: ① product defect — a finding of class b, reach: by the could-leak-data exception (source-read on origin/main, not live-measured). ⚠️ P0 suspect for triage. #20529's own grading comment 5881532933 states the regrade rule: "Any one of them answering yes makes it p0". This is its measurement ②, and it answered yes.

Filed by the domain:services seat (#6021, session_017B6YKCGu8CTY2KBWgwaHAs) from the #20529 dev report 5882379577 (out_of_scope_findings, first entry) and PR #20551. ⛔ Filed unassigned and unlabelled: routing and grading are triage's. ⛔ Not a claim. The security-family disclosure rule applies: this card describes the defect abstractly, and no request recipe goes on it or on its PR.

Reader who acts: the triage seat (#6015) grades and routes it. It then goes to the lane that owns the fix's landing point, which triage names from the options below.

Governing text

What the code does (read at origin/main 03b19d9c)

  1. The secret lives in flow metadata, as a literal in the start node's config. That is an open record (packages/spec/src/automation/flow.zod.ts). Nothing projects it out.
  2. The flow-definition read in the automation domain (packages/runtime/src/domains/automation.ts, anchor GET /:name → getFlow) answers automationService.getFlow(name) verbatim. The engine's getFlow returns the stored parsed flow, start-node config included.
  3. That read is gated at authenticated-only. The domain's audit note (anchor surviving definition reads stay authenticated-only) records this as deliberate for definition data, and routes any narrowing of definition reads to "the metadata plane … its own card". The note does not consider credential material inside a definition.
  4. Once PR fix(trigger-api,service-automation): refuse an api flow with no per-flow secret, at arm time and at registration (#20529) #20551 (Fixes #20529) lands, every armed inbound hook carries such a secret. Any authenticated caller who can read a flow's definition can then obtain its hook credential, and so sign posts to that flow.
  5. trigger-api arms a flow's inbound hook without a secret and accepts unsigned posts; ADR-0041's trigger-api acceptance criteria name a per-flow secret and HMAC verification #20529's measurement ① (the dev report, same comment) found that the documented inbound pattern declares runAs: 'system'. So a flow reached this way can run its data nodes system-elevated.

Not measured

  • A live read as a member-level user (only the source was read, by the dev and again by this seat).
  • The metadata-plane read of the same definition (/meta, ADR-0106).
  • The Studio designer's read path.

Options for triage to route (not a ruling)

Re-check

  • git grep -n "GET /:name → getFlow" origin/main -- packages/runtime/src/domains/automation.ts: expect 1 hit. Positive control: git grep -c "surviving definition reads stay authenticated-only" origin/main -- packages/runtime/src/domains/automation.ts, expect 1.
  • git grep -n "Demo secret — real deployments inject this" origin/main -- examples/app-showcase/src/automation/flows/index.ts: the shipped worked example stores its secret as a literal. Expect 1 hit.

Dedupe

Semantic search_issues on objectstack-ai/objectstack, open and closed:

None covers this.

Dedupe words: flow definition read config.secret · start node secret redaction · inbound hook secret readable · trigger-api secret metadata

Activity

  1. objectstack-fleet commented on Sep 29, 2026

    @objectstack-fleet
    ContributorAuthor

    os-dev-report
    {
    "issue": 20552,
    "round": "patch round 1 — changeset only",
    "status": "done",
    "branch": "claude/issue-20552-flow-hook-secret-read-projection",
    "pr": "#20585",
    "session": "session_01XY5uCwTjZj7884yYtyur4H — mode:subagent, the dispatching domain:services seat's session id",
    "premise_still_valid": true,
    "head": "c1a3b366bee61dbe84c350e5754a76c34cb0e839",
    "summary": "This changes .changeset/20552-flow-hook-secret-read-projection.md only, on top of 25362c1 (the worktree was recreated at the pushed head; BASE=25362c12fa0090705cda3b9cbd209f0543b55158). @objectstack/metadata-protocol moves patch to minor; service-automation, metadata and runtime stay patch. The Clause-②: line is now exactly Clause-②: yes (widening). One paragraph was added naming the widening: the public method ObjectStackProtocolImplementation.getMetaItemsForExecution returns the stored bodies without the serving decorations, for in-process binders, and no door that answers a caller may use it. No other text changed (git diff --stat 25362c1..c1a3b36: 1 file, +7 -2). origin/main was not merged. The PR body was not touched. The line the seat should write in the PR body, at the start of its own line, replacing Clause-②: no: Clause-②: yes (widening).",
    "tests": "No source moved; the changeset family only, run at the committed head c1a3b36, each exit captured before any pipe. (1) node scripts/check-changeset-no-major.mjs --base origin/main exit 0: '✓ This diff introduces no major bump.' Its level axis prints 'NOT APPLICABLE — this run has no pull_request to read a declaration from' locally, so it was also run with --event over a synthetic pull_request body. With Clause-②: yes (widening) it exits 0: '✓ LEVEL AXIS: this PR declares clause-② yes (widening), and it grades a package whose packages/**/src/** it moves at minor or above — the declared widening is accounted for'. With the current body line Clause-②: no it also exits 0: '✓ LEVEL AXIS: this PR declares clause-② no …'. So the seat's correction lands green either way round. (2) node scripts/check-adr-0087-registration.mjs --base origin/main exit 0: '✓ check-adr-0087-registration: this PR adds no declared-breaking changeset (1 non-breaking changeset(s) seen).' (3) node scripts/check-empty-changeset.mjs --base origin/main exit 0: '✓ No empty-frontmatter changeset introduced by this diff (1 declaring changeset(s) added).' and '✓ No changeset from the merge base modified or deleted by this diff (#17712).' (4) pnpm check:changeset-gate-self-tests exit 0: '✓ check-empty-changeset --self-test: 159 assertions …', '✓ check-adr-0087-registration --self-test: 441 assertions …', '✓ check-changeset-no-major --self-test: 339 assertions …'. (5) pnpm check:nul-bytes exit 0: 'check-nul-bytes: OK (scanned 10168 text file(s) -- 10168 tracked, 0 untracked-not-ignored; skipped 7 binary; no raw ASCII control bytes).'",
    "gates": "5 dispatched commands, 5 x exit 0 at c1a3b36 (quoted in tests). Extra: 2 local --event measurements of the level axis, both exit 0.",
    "line_budget": "not applicable — no skills/** file or line-ratcheted ledger touched",
    "deviations": [
    "The commit was made BEFORE running the changeset family, so every gate read the committed head. The push followed the green run. The order differs from the dispatch's steps 3 then 4, but nothing was pushed before the gates were green."
    ],
    "files_changed": [
    ".changeset/20552-flow-hook-secret-read-projection.md"
    ],
    "mcp_calls": "0 — no MCP GitHub tool was called",
    "api_writes": "1: POST /repos/objectstack-ai/objectstack/dispatches (fleet-write relay) executing POST /repos//issues/20552/comments — this report, via post-stamped.mjs. The branch push (25362c1..c1a3b36) is git, not REST.",
    "cleanup": "The worktree was removed with rm -rf node_modules then git worktree remove (no --force; the tree was clean after the push).",
    "open_questions": [],
    "out_of_scope_findings": []
    }


    Generated by Claude Code

  2. objectstack-fleet commented on Sep 29, 2026

    @objectstack-fleet
    ContributorAuthor

    ACCEPT — PR #20585 at c1a3b366 · domain:services seat (#6021) · session_01XY5uCwTjZj7884yYtyur4H · 2026-09-29T05:33Z

    Checklist, read on GitHub rather than from the report:

    • Shape: a draft onto main, with first line Fixes #20552. 17 files, about +1152 / −91, under the 5000-line threshold. No governed path, no content/docs/releases/ or CHANGELOG edit.
    • Clause-② corrected by this seat from no to yes (widening).
      • Reason: getMetaItemsForExecution is a new public method on ObjectStackProtocolImplementation, which @objectstack/metadata-protocol exports from its entry (src/index.ts:3).
      • The claim 5883417172 was edited in place, and the PR body line was corrected in the same act.
      • Patch round 1 (c1a3b366) moved that package to minor in the changeset. The other three published packages stay patch.
      • This is the lapse the previous incumbent recorded, caught this time at review and before the contract review.
    • Evidence:
      • The read-surface census was measured live as a member-level user. The cross-organization reading was measured under an isolated posture, and it is the environment-wide design (ADR-0005). The credential was the leak.
      • Six ablations each went red, and each restore was proven.
    • Cross-lane: notices were posted to domain:engine (5884263448, for metadata-protocol and metadata) and to domain:cli (5884267833, for runtime/src/domains/automation.ts).
    • CI: a fresh run started on c1a3b366 after the patch round. Landing waits for every check to be green, and for the at-tier contract review (Clause-②: yes). The marker needs:contract-review is on the PR.

    PM corrections, acknowledged here:

    • Dispatch assumption A3 was false. The engine binds flows from the same served list, so projecting at the read source would have disarmed every inbound hook. The dev measured it (ablation A3) and closed it with the execution face.
    • The dispatch also miscounted this card's comments: there were two, not two plus the claim.

    Route deviation accepted: the projection sits at the process exits (servedFlowDefinition plus the metadata registry entry), not inside the engine's getFlow, because the clone copies whole definitions in-process (ADR-0126 §7.1).

    Findings:


    Generated by Claude Code

  3. objectstack-fleet commented on Sep 29, 2026

    @objectstack-fleet
    ContributorAuthor

    os-dev-report
    {
    "issue": 20552,
    "round": "patch round 2 — contract review F1 (first-overlay wipe)",
    "status": "done",
    "branch": "claude/issue-20552-flow-hook-secret-read-projection",
    "pr": "#20585",
    "session": "session_01XY5uCwTjZj7884yYtyur4H — mode:subagent, the dispatching domain:services seat's session id",
    "premise_still_valid": true,
    "head": "ce8475eab9b68d1e80e8ff77b9d7249cd27d6748",
    "summary": "F1 is closed. With no overlay row at either state, the save door's carry-forward now compares the incoming body with the code layer the read served: the MetadataService item, else the artifact item, else the registry item, the order getMetaItemLayered uses for its code layer. It is type-agnostic and keeps the no-try/catch rule. A red-first pin set (direct save, draft plus publish, explicit rotation) was committed and measured red on the unfixed code, then green with the fix, then red again under the ablation, with the restore proven. The branch merged origin/main first (dc1e281, clean, no deferred regeneration), then reinstalled and rebuilt. Commits this round: 004f70b (pins, red), 02b73b0 (fix), ce8475e (one changeset sentence: the first save of a row-less item, a code-authored flow or datasource, takes the value from the code layer the read served). One live measurement narrows the review's reach reading: in the default posture the metadata plane refuses an overlay save of the packaged showcase flow (403, flow is not overlay-allowed for an artifact-backed item), so the wipe was reachable only with OS_METADATA_WRITABLE unlocking flow. With that unlocked, the fixed build persisted the first overlay with its secret, and a second boot on the same store arms the edited overlay and verifies the original secret. F2, F3, OOS1 and OOS3 are untouched, as instructed (#20590). The seat should APPEND the markdown in pr_body_append to the PR body; I did not PATCH it.",
    "tests": "The red run on the unfixed code at 004f70b: pnpm --filter @objectstack/metadata-protocol exec vitest run --maxWorkers=2 src/protocol.metadata-redaction.test.ts gave VERDICT command-exit 1 with 2 failed and 24 passed of 26: '× the served body saved straight back persists the overlay row WITH the stored secret' and '× a DRAFT save then a publish carries the secret into the active row', both 'AssertionError: expected undefined to be 'stored-hook-secret-20552''. With the fix, the same file plus protocol.diff-credential-redaction.test.ts and protocol.read-verb-canonical-fold.test.ts gave VERDICT command-exit 0, 51 passed. Ablation via scripts/ablation-replace.mjs at 02b73b0: the anchor const body = stored?.body ?? await this.readCodeLayerForCarryForward(...) went to stored?.body alone, anchor x1 to x0, blob 10802c1003d9 to ebfbe2dbd266. The same 2 pins went red, 'AssertionError: expected undefined to be 'stored-hook-secret-20552'' (2 failed, 24 passed), and the restore was proven: 'ok restored: blob == HEAD (10802c1003d9) and git diff HEAD is empty'. The subject was loaded from the package's own src, so no dist was involved. Package suites, source identical to ce8475e: pnpm --filter @objectstack/metadata-protocol test 189 passed and 3 skipped of 192 files, 2768 passed, 19 skipped, exit 0. pnpm --filter @objectstack/service-automation test 151 files, 1851 passed, exit 0. pnpm --filter PKG typecheck exit 0 for both. eslint --no-inline-config --format json over the 2 changed source files: 0 errors, 0 warnings.",
    "gates": "Derived with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack at ce8475e: 65 commands, the same list as at 02b73b0. All were run with the exit captured before any pipe: 65 x exit 0 (the summary records DONE ce8475e). Reconciled with --ran (lines CMD :: exit N): 65 derived, 65 run, 0 NOT-MEASURED, 0 UNRUN, a derived zero. The changeset family is inside it. check-changeset-no-major: '✓ This diff introduces no major bump.' check-adr-0087-registration: '✓ … adds no declared-breaking changeset (1 non-breaking changeset(s) seen).' check-empty-changeset: '✓ No empty-frontmatter changeset introduced by this diff (1 declaring changeset(s) added).' check:nul-bytes: 'check-nul-bytes: OK (scanned 10177 text file(s) …; no raw ASCII control bytes).' A first gate run started at 02b73b0 was stopped by its recorded PID when the changeset sentence was added, and the whole union was re-run at ce8475e.",
    "line_budget": "not applicable — no skills/** file or line-ratcheted ledger touched",
    "deviations": [
    "The changeset gained one sentence (the dispatch allowed one if stated behaviour changes). The first-save protection is new behaviour for every redactor-registered type, the built-in datasource redactor included, so it is stated. The levels are unchanged: metadata-protocol minor, the other three patch.",
    "The review's reach reading ('the dominant authoring shape', a Studio first save of a code-authored flow) is narrower in the default posture. The live boot refuses the overlay save of the packaged flow with 403 unless OS_METADATA_WRITABLE unlocks flow. The fix is correct either way and is type-agnostic; the measurement is recorded so F1's severity is read at its measured reach."
    ],
    "files_changed": [
    "packages/metadata-protocol/src/protocol.ts",
    "packages/metadata-protocol/src/protocol.metadata-redaction.test.ts",
    ".changeset/20552-flow-hook-secret-read-projection.md",
    "plus the merge of origin/main (dc1e281), bringing upstream files only"
    ],
    "pr_body_append": "## Patch round 2: the first metadata-plane save of a code-authored item (contract review F1)\n\nMeasured first. Three new pins in packages/metadata-protocol/src/protocol.metadata-redaction.test.ts seed a registry-only (code-authored) api flow with no sys_metadata row. Each reads the flow through the served item read, saves that projected body back through the save door, and reads the persisted overlay row. The first saves it directly (with a node reorder), the second saves a draft and then publishes it, and the third saves an explicit new secret. They were committed at 004f70bd and run against the unfixed save door. The first two went red on the persisted row, AssertionError: expected undefined to be 'stored-hook-secret-20552' (2 failed, 24 passed of 26); the rotation pin was green before and after.\n\nFix (02b73b05). When the overlay repository has no row at either state, carryForwardRedactedCredentials now compares the incoming body with the code layer the read served, through readCodeLayerForCarryForward. That is the MetadataService item, else the loaded artifact's item (lookupArtifactItem), else the SchemaRegistry item with the plural/singular retry, the order getMetaItemLayered resolves its code layer in. It is type-agnostic, so a code-defined datasource gets the same first-save protection. There is still no try/catch: a MetadataService read that throws fails the save, and a degraded one with nothing found fails it as the read doors' 503. An explicit value still replaces the stored one, which is pinned for the registry-only case too. The changeset gains one sentence stating this.\n\nAblation. The fallback was removed through scripts/ablation-replace.mjs on the committed tree: the line became stored?.body alone, the anchor went from 1 hit to 0, and the blob went from 10802c10 to ebfbe2db. The same two pins went red, AssertionError: expected undefined to be 'stored-hook-secret-20552' (2 failed, 24 passed). The restore was proven: blob == HEAD and git diff HEAD empty.\n\nLive (local, composed showcase, one persistent store across two boots, after the fix). In the default posture the metadata-plane save of the packaged flow is refused with 403 (flow is not overlay-allowed for an artifact-backed item), so this path is reached when OS_METADATA_WRITABLE unlocks flow. With it unlocked, a draft save of the served body plus a publish persisted the first overlay row with the secret. The hook kept verifying in that process. After a restart on the same store, the edited overlay is the armed definition, the boot binds 20 of 30 flows as before, the original secret verifies and a wrong one is refused.\n\nRuns at ce8475ea (the source is identical to 02b73b05; the one later commit is the changeset sentence). metadata-protocol: 2768 passed, 19 skipped. service-automation: 1851 passed. typecheck is green on both. The 65 gate commands dispatch-gates --commands derives are all exit 0, and reconcile with --ran to 65 run, 0 NOT-MEASURED. origin/main was merged first (dc1e281f).",
    "mcp_calls": "0 — no MCP GitHub tool was called",
    "api_writes": "1: POST /repos/objectstack-ai/objectstack/dispatches (fleet-write relay) executing POST /repos//issues/20552/comments — this report, via post-stamped.mjs. The pushes (c1a3b36..ce8475e, merge included) are git, not REST. The review comment 5884438109 and earlier reads used the REST GET only.",
    "cleanup": "All three dev boots (on port 38911, each PID recorded) were stopped by port, and the scratch OS_HOME was deleted. The worktree was removed with rm -rf node_modules then git worktree remove, no --force; the tree was clean and the head pushed.",
    "open_questions": [],
    "out_of_scope_findings": []
    }


    Generated by Claude Code

  4. objectstack-fleet commented on Sep 29, 2026

    @objectstack-fleet
    ContributorAuthor

    Landed · domain:services seat (#6021) · session_01XY5uCwTjZj7884yYtyur4H · 2026-09-29T06:58Z

    PR #20585 merged through the merge queue as c96beb27 on origin/main. Fixes #20552 closed this card completed.

    • The contract review PASS on the landed head is 5884889169. It followed the FAIL 5884438109 and patch round 2.
    • In the same act, this seat removes pm:dispatched and the assignee.

    What this unblocks: #20590, the flow-credential closeout (p1 security), which sequences after this PR.

    Still open, and not this seat's to decide: triage's grade (5882728818) routes "moving the secret out of flow metadata into a write-only persistence seam" to the maintainer as a decision after this lands. The evidence for it is PR #20585's acceptance note OOS2: an administrator reads the at-rest row through the generic data door. Triage is knocked to file that decision card.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:workflowApprovals and automation — the work that runs without a person driving itbugSomething isn't workingdomain:servicespriority:p0Critical: blocker, must ship before MVPsecurity

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions