ci(secrets->kms): migrate CP_ADMIN_API_TOKEN to Infisical KMS (wave-3 molecule-ai/molecule-core) #3274

Merged
agent-reviewer-cr2 merged 3 commits from ci/migrate-cp-admin-api-token-infisical into main 2026-06-25 22:06:46 +00:00
Owner

What

Migrate CP_ADMIN_API_TOKEN from a duplicated Gitea Actions secret to the Infisical KMS SSOT across the 7 molecule-ai/molecule-core workflows that consume it. Wave-3 of the Gitea-secrets-to-KMS migration; one secret per PR (established wave pattern).

Recipe (mirrors bench-provision-time.yml + boot-to-registration-e2e.yml)

In the same job as each consumer (so the export spans the steps via $GITHUB_ENV), a Fetch CP_ADMIN_API_TOKEN from Infisical KMS step:

  1. universal-auth login POST https://key.moleculesai.app/api/v1/auth/universal-auth/login with the CI machine identity (INFISICAL_CI_CLIENT_ID / INFISICAL_CI_CLIENT_SECRET, project INFISICAL_CI_PROJECT_ID) -> accessToken.
  2. GET /api/v3/secrets/raw/CP_ADMIN_API_TOKEN?workspaceId=...&environment=prod&secretPath=%2Fshared%2Fcontrolplane-admin -> .secret.secretValue.
  3. Masking-control: echo "::add-mask::$VAL", then assert NON-EMPTY and FAIL LOUD ([ -z "$VAL" ] -> ::error:: + exit 1), then echo "CP_ADMIN_API_TOKEN=$VAL" >> "$GITHUB_ENV".
  4. Every secrets.CP_ADMIN_API_TOKEN use site now consumes the env var. template-delivery-e2e.yml keeps its MOLECULE_ADMIN_TOKEN alias (exported under that name).

The 3 INFISICAL_CI_* bootstrap secrets STAY in Gitea -- they are the identity that fetches everything else.

Trusted-ref gate (this repo is PUBLIC)

The KMS step consumes the INFISICAL_CI_* machine-identity secrets, so it is gated to trusted refs so an untrusted fork cannot trigger it and harvest those creds. Per-workflow if::

Workflow Job Triggers KMS-step gate
template-delivery-e2e.yml delivery wf_dispatch, push, pull_request delivery=='true' AND (workflow_dispatch OR push+main OR pull_request with head.repo.fork == false)
staging-verify.yml promote-to-latest push:staging, wf_dispatch workflow_dispatch OR push+staging
redeploy-tenants-on-main.yml redeploy wf_dispatch workflow_dispatch
sweep-cf-tunnels.yml sweep schedule schedule OR workflow_dispatch
sweep-cf-orphans.yml sweep schedule schedule OR workflow_dispatch
sweep-aws-secrets.yml sweep schedule, wf_dispatch schedule OR workflow_dispatch
publish-workspace-server-image.yml deploy-production push:main, wf_dispatch workflow_dispatch OR push+main

For template-delivery-e2e a same-repo (non-fork) delivery PR runs the fetch + e2e normally; a fork delivery PR skips the fetch and then hits the existing "Verify required secret present" hard-fail (loud, not silent) -- fail-closed, and equivalent to today since forks cannot read secrets.* in this Gitea setup anyway.

No on: trigger changes in any file (wave-1 lesson: a migration PR must not alter triggers).

Probe-200 evidence

GET /v3/secrets/raw/CP_ADMIN_API_TOKEN?...&secretPath=%2Fshared%2Fcontrolplane-admin probed 200 before shipping:

  • environment=prod -> 200, len 64
  • environment=staging -> 200, len 64 (byte-identical value across envs)

Path pinned to /shared/controlplane-admin (the assessment's /shared/cp-admin is 404 -- probe-first caught it).

Validation

  • yaml.safe_load passes on all 7 changed files; on: blocks and jobs intact.
  • Repo lint-workflow-yaml.py --workflow-dir .gitea/workflows -> 0 warnings, no Gitea-1.22.6-hostile shapes.

Gitea CP_ADMIN_API_TOKEN secret RETAINED pending validate-before-delete

The Gitea CP_ADMIN_API_TOKEN secret is NOT deleted by this PR. Per the wave gate, the PM/operator re-runs a real workflow on this PR first to prove the KMS fetch is live AND consumed; only then is the Gitea secret retired (repo scope; org scope only after ALL consumers migrate).

Merge discipline

No self-merge, no self-approval. Requires the standard pool review bar.

## What Migrate `CP_ADMIN_API_TOKEN` from a duplicated Gitea Actions secret to the Infisical KMS SSOT across the 7 `molecule-ai/molecule-core` workflows that consume it. Wave-3 of the Gitea-secrets-to-KMS migration; one secret per PR (established wave pattern). ## Recipe (mirrors bench-provision-time.yml + boot-to-registration-e2e.yml) In the same job as each consumer (so the export spans the steps via `$GITHUB_ENV`), a `Fetch CP_ADMIN_API_TOKEN from Infisical KMS` step: 1. universal-auth login `POST https://key.moleculesai.app/api/v1/auth/universal-auth/login` with the CI machine identity (`INFISICAL_CI_CLIENT_ID` / `INFISICAL_CI_CLIENT_SECRET`, project `INFISICAL_CI_PROJECT_ID`) -> accessToken. 2. `GET /api/v3/secrets/raw/CP_ADMIN_API_TOKEN?workspaceId=...&environment=prod&secretPath=%2Fshared%2Fcontrolplane-admin` -> `.secret.secretValue`. 3. Masking-control: `echo "::add-mask::$VAL"`, then assert NON-EMPTY and FAIL LOUD (`[ -z "$VAL" ]` -> `::error::` + `exit 1`), then `echo "CP_ADMIN_API_TOKEN=$VAL" >> "$GITHUB_ENV"`. 4. Every `secrets.CP_ADMIN_API_TOKEN` use site now consumes the env var. `template-delivery-e2e.yml` keeps its `MOLECULE_ADMIN_TOKEN` alias (exported under that name). The 3 `INFISICAL_CI_*` bootstrap secrets STAY in Gitea -- they are the identity that fetches everything else. ## Trusted-ref gate (this repo is PUBLIC) The KMS step consumes the `INFISICAL_CI_*` machine-identity secrets, so it is gated to trusted refs so an untrusted fork cannot trigger it and harvest those creds. Per-workflow `if:`: | Workflow | Job | Triggers | KMS-step gate | |---|---|---|---| | `template-delivery-e2e.yml` | `delivery` | wf_dispatch, push, pull_request | `delivery=='true'` AND (`workflow_dispatch` OR `push`+`main` OR `pull_request` with `head.repo.fork == false`) | | `staging-verify.yml` | `promote-to-latest` | push:staging, wf_dispatch | `workflow_dispatch` OR `push`+`staging` | | `redeploy-tenants-on-main.yml` | `redeploy` | wf_dispatch | `workflow_dispatch` | | `sweep-cf-tunnels.yml` | `sweep` | schedule | `schedule` OR `workflow_dispatch` | | `sweep-cf-orphans.yml` | `sweep` | schedule | `schedule` OR `workflow_dispatch` | | `sweep-aws-secrets.yml` | `sweep` | schedule, wf_dispatch | `schedule` OR `workflow_dispatch` | | `publish-workspace-server-image.yml` | `deploy-production` | push:main, wf_dispatch | `workflow_dispatch` OR `push`+`main` | For `template-delivery-e2e` a same-repo (non-fork) delivery PR runs the fetch + e2e normally; a fork delivery PR skips the fetch and then hits the existing "Verify required secret present" hard-fail (loud, not silent) -- fail-closed, and equivalent to today since forks cannot read `secrets.*` in this Gitea setup anyway. No `on:` trigger changes in any file (wave-1 lesson: a migration PR must not alter triggers). ## Probe-200 evidence `GET /v3/secrets/raw/CP_ADMIN_API_TOKEN?...&secretPath=%2Fshared%2Fcontrolplane-admin` probed 200 before shipping: - `environment=prod` -> 200, len 64 - `environment=staging` -> 200, len 64 (byte-identical value across envs) Path pinned to `/shared/controlplane-admin` (the assessment's `/shared/cp-admin` is 404 -- probe-first caught it). ## Validation - `yaml.safe_load` passes on all 7 changed files; `on:` blocks and jobs intact. - Repo `lint-workflow-yaml.py --workflow-dir .gitea/workflows` -> 0 warnings, no Gitea-1.22.6-hostile shapes. ## Gitea CP_ADMIN_API_TOKEN secret RETAINED pending validate-before-delete The Gitea `CP_ADMIN_API_TOKEN` secret is NOT deleted by this PR. Per the wave gate, the PM/operator re-runs a real workflow on this PR first to prove the KMS fetch is live AND consumed; only then is the Gitea secret retired (repo scope; org scope only after ALL consumers migrate). ## Merge discipline No self-merge, no self-approval. Requires the standard pool review bar.
hongming added 1 commit 2026-06-25 21:40:38 +00:00
ci(secrets->kms): migrate CP_ADMIN_API_TOKEN to Infisical KMS (wave-3 molecule-ai/molecule-core)
CI / Python Lint & Test (pull_request) Successful in 6s
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 7s
Block integration-tester contamination artifacts / Block staging-trigger / invalid manifest contamination (pull_request) Successful in 8s
Lint forbidden tenant-env keys / Scan workspace_secrets writers for forbidden env keys (pull_request) Successful in 6s
Lint curl status-code capture / Scan workflows for curl status-capture pollution (pull_request) Successful in 7s
Lint forbidden tenant-env keys / Scan for repo-host token write into tenant workspace surface (pull_request) Successful in 6s
E2E Peer Visibility (literal MCP list_peers) / detect-changes (pull_request) Successful in 13s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 12s
CI / Detect changes (pull_request) Successful in 17s
E2E Chat / detect-changes (pull_request) Successful in 20s
lint-required-workflows-docker-host-pinned / Lint docker-host pin on docker-touching workflows (pull_request) Successful in 6s
E2E API Smoke Test / detect-changes (pull_request) Successful in 22s
Lint publish-runner timeout-minutes / Lint publish-runner timeout-minutes (pull_request) Successful in 13s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 16s
lint-setup-go-cache / lint-setup-go-cache (pull_request) Successful in 14s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 8s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 28s
sop-checklist / review-refire (pull_request_target) Has been skipped
lint-no-coe-on-required / lint-no-coe-on-required (pull_request) Successful in 26s
Lint workflow YAML (Gitea-1.22.6-hostile shapes) / Lint workflow YAML for Gitea-1.22.6-hostile shapes (pull_request) Successful in 17s
CI / Canvas (Next.js) (pull_request) Successful in 2s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 2s
E2E Peer Visibility (literal MCP list_peers) / E2E Peer Visibility (local) (pull_request) Has been skipped
CI / Platform (Go) (pull_request) Successful in 3s
reserved-path-review / reserved-path-review (pull_request_target) Failing after 9s
qa-review / approved (pull_request_target) Failing after 9s
gate-check-v3 / gate-check (pull_request_target) Successful in 14s
security-review / approved (pull_request_target) Failing after 8s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 3s
PR Diff Guard / PR diff guard (pull_request) Successful in 17s
sop-checklist / na-declarations (pull_request) N/A: (none)
lint-continue-on-error-tracking / lint-continue-on-error-tracking (pull_request) Successful in 35s
E2E Chat / E2E Chat (pull_request) Successful in 4s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 3s
E2E Peer Visibility (literal MCP list_peers) / E2E Peer Visibility (pull_request) Successful in 6s
sop-checklist / all-items-acked (pull_request_target) Successful in 10s
CI / Canvas Deploy Status (pull_request) Successful in 1s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 3s
CI / all-required (pull_request) Successful in 3s
Lint pre-flip continue-on-error / Verify continue-on-error flips have run-log proof (pull_request) Successful in 37s
template-delivery-e2e / detect-changes (pull_request) Successful in 32s
Local Provision Lifecycle E2E / Local Provision Lifecycle E2E (stub) (pull_request) Successful in 37s
lint-required-context-exists-in-bp / lint-required-context-exists-in-bp (pull_request) Successful in 55s
Local Provision Lifecycle E2E / Local Provision Lifecycle E2E (real image + MiniMax LLM, advisory) (pull_request) Successful in 37s
E2E Staging SaaS (full lifecycle) / pr-validate (pull_request) Has been cancelled
E2E Staging SaaS (full lifecycle) / E2E Staging SaaS (pull_request) Has been cancelled
E2E Staging SaaS (full lifecycle) / Prune stale e2e DNS records (pull_request) Has been cancelled
E2E Staging SaaS (full lifecycle) / E2E Staging Platform Boot (pull_request) Has been cancelled
E2E Staging SaaS (full lifecycle) / E2E Staging Concierge user_tasks (pull_request) Has been cancelled
E2E Staging SaaS (full lifecycle) / E2E Staging Workspace Requests (core#2606) (pull_request) Has been cancelled
E2E Staging SaaS (full lifecycle) / E2E Staging Concierge Creates Workspace (pull_request) Has been cancelled
E2E Staging SaaS (full lifecycle) / E2E Staging Concierge (compile+skip) (pull_request) Has been cancelled
E2E Staging SaaS (full lifecycle) / E2E Staging Concierge Platform Agent (pull_request) Has been cancelled
E2E Staging SaaS (full lifecycle) / E2E Staging Plugin Install Lifecycle (pull_request) Has been cancelled
template-delivery-e2e / Template-asset delivery (fresh seo-agent — config+prompts via asset channel, seo-all via plugin reconcile) (pull_request) Successful in 11m50s
sop-checklist / all-items-acked (pull_request) Compensated by status-reaper (non-required pull_request/pull_request_review governance shadow overridden by successful pull_request_target status; see .gitea/scripts/status-reaper.py)
8e821e1fc0
Source CP_ADMIN_API_TOKEN from the Infisical SSOT (prod /shared/controlplane-admin,
probe 200, len 64) via the CI machine identity (INFISICAL_CI_CLIENT_ID/SECRET +
INFISICAL_CI_PROJECT_ID) instead of the duplicated Gitea Actions secret, in the 7
molecule-core workflows that consume it. Mirrors the proven bench-provision-time.yml
and boot-to-registration-e2e.yml KMS-fetch pattern.

Per workflow, in the SAME job as the consumer (so $GITHUB_ENV spans it):
- add a "Fetch CP_ADMIN_API_TOKEN from Infisical KMS" step: universal-auth login ->
  GET /api/v3/secrets/raw/CP_ADMIN_API_TOKEN?environment=prod&secretPath=
  %2Fshared%2Fcontrolplane-admin -> ::add-mask:: -> assert NON-EMPTY + FAIL LOUD ->
  export to $GITHUB_ENV;
- replace every secrets.CP_ADMIN_API_TOKEN use site with the env var (template-
  delivery-e2e keeps its MOLECULE_ADMIN_TOKEN alias, exported under that name);
- GATE the KMS step to TRUSTED refs (push/workflow_dispatch/schedule, or a same-repo
  non-fork pull_request with head.repo.fork == false) so untrusted forks cannot
  trigger it and harvest INFISICAL_CI_*.

No on: trigger changes. The 3 INFISICAL_CI_* bootstrap secrets stay in Gitea (they
are the identity that fetches everything else). The Gitea CP_ADMIN_API_TOKEN secret
is RETAINED pending validate-before-delete.

Files:
- template-delivery-e2e.yml (delivery; wf_dispatch/push/PR; trusted = same-repo PR)
- staging-verify.yml (promote-to-latest; push:staging/wf_dispatch)
- redeploy-tenants-on-main.yml (redeploy; wf_dispatch)
- sweep-cf-tunnels.yml (sweep; schedule)
- sweep-cf-orphans.yml (sweep; schedule)
- sweep-aws-secrets.yml (sweep; schedule/wf_dispatch)
- publish-workspace-server-image.yml (deploy-production; push:main/wf_dispatch)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
hongming added 1 commit 2026-06-25 21:51:47 +00:00
ci(security): fail-close trusted-ref gate on template-delivery-e2e KMS step
CI / Python Lint & Test (pull_request) Successful in 6s
Block integration-tester contamination artifacts / Block staging-trigger / invalid manifest contamination (pull_request) Successful in 8s
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 8s
Lint curl status-code capture / Scan workflows for curl status-capture pollution (pull_request) Successful in 7s
Lint forbidden tenant-env keys / Scan for repo-host token write into tenant workspace surface (pull_request) Successful in 6s
Lint forbidden tenant-env keys / Scan workspace_secrets writers for forbidden env keys (pull_request) Successful in 7s
E2E Peer Visibility (literal MCP list_peers) / detect-changes (pull_request) Successful in 13s
lint-required-workflows-docker-host-pinned / Lint docker-host pin on docker-touching workflows (pull_request) Successful in 7s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 9s
Lint publish-runner timeout-minutes / Lint publish-runner timeout-minutes (pull_request) Successful in 17s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 18s
lint-no-coe-on-required / lint-no-coe-on-required (pull_request) Successful in 19s
sop-checklist / review-refire (pull_request_target) Has been skipped
E2E Peer Visibility (literal MCP list_peers) / E2E Peer Visibility (local) (pull_request) Has been skipped
lint-setup-go-cache / lint-setup-go-cache (pull_request) Successful in 19s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 25s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 30s
gate-check-v3 / gate-check (pull_request_target) Failing after 16s
sop-checklist / all-items-acked (pull_request) acked: 0/9 — missing: comprehensive-testing, local-postgres-e2e, staging-smoke, +6 — body-unfilled: comprehensive-testing, local-postgres-e2
CI / Detect changes (pull_request) Successful in 34s
E2E Peer Visibility (literal MCP list_peers) / E2E Peer Visibility (pull_request) Successful in 6s
sop-checklist / na-declarations (pull_request) N/A: (none)
Lint workflow YAML (Gitea-1.22.6-hostile shapes) / Lint workflow YAML for Gitea-1.22.6-hostile shapes (pull_request) Successful in 22s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 3s
sop-checklist / all-items-acked (pull_request_target) Successful in 9s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 4s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 2s
CI / Canvas (Next.js) (pull_request) Successful in 3s
E2E API Smoke Test / detect-changes (pull_request) Successful in 39s
CI / Platform (Go) (pull_request) Successful in 4s
CI / Canvas Deploy Status (pull_request) Successful in 1s
template-delivery-e2e / detect-changes (pull_request) Successful in 26s
lint-continue-on-error-tracking / lint-continue-on-error-tracking (pull_request) Successful in 37s
E2E Chat / detect-changes (pull_request) Successful in 42s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 3s
PR Diff Guard / PR diff guard (pull_request) Successful in 31s
CI / all-required (pull_request) Successful in 3s
E2E Chat / E2E Chat (pull_request) Successful in 4s
Lint pre-flip continue-on-error / Verify continue-on-error flips have run-log proof (pull_request) Successful in 42s
lint-required-context-exists-in-bp / lint-required-context-exists-in-bp (pull_request) Successful in 43s
Local Provision Lifecycle E2E / Local Provision Lifecycle E2E (stub) (pull_request) Successful in 38s
Local Provision Lifecycle E2E / Local Provision Lifecycle E2E (real image + MiniMax LLM, advisory) (pull_request) Successful in 2m10s
qa-review / approved (pull_request_target) Review check failed via pull_request_review trigger
reserved-path-review / reserved-path-review (pull_request_target) Review check failed via pull_request_review trigger
qa-review / approved (pull_request_review) Failing after 10s
reserved-path-review / reserved-path-review (pull_request_review) Failing after 11s
security-review / approved (pull_request_target) Review check failed via pull_request_review trigger
security-review / approved (pull_request_review) Failing after 12s
E2E Staging SaaS (full lifecycle) / E2E Staging Platform Boot (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / E2E Staging Concierge user_tasks (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / E2E Staging Workspace Requests (core#2606) (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / E2E Staging Concierge Creates Workspace (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / E2E Staging Concierge (compile+skip) (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / E2E Staging Concierge Platform Agent (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / E2E Staging Plugin Install Lifecycle (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / pr-validate (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / E2E Staging SaaS (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / Prune stale e2e DNS records (pull_request) Blocked by required conditions
template-delivery-e2e / Template-asset delivery (fresh seo-agent — config+prompts via asset channel, seo-all via plugin reconcile) (pull_request) Successful in 9m15s
857eb6caad
The 'Fetch CP_ADMIN_API_TOKEN from Infisical KMS' step's pull_request fork
clause used `github.event.pull_request.head.repo.fork == false`, which
FAIL-OPENS in the Actions expression engine: when Gitea's pull_request event
omits/nulls head.repo.fork, `null == false` coerces TRUE, so a FORK PR would
run the step and could exfiltrate INFISICAL_CI_CLIENT_ID/SECRET/PROJECT_ID.
This is a PUBLIC-repo, always-running REQUIRED PR gate (on: pull_request
branches:[main], delivery job has no job-level if:), so it was exploitable.

Replace the negative fork check with the FAIL-CLOSED positive same-repo
allowlist `github.event.pull_request.head.repo.full_name == github.repository`,
which admits ONLY a trusted same-repo head and fails closed on a fork,
cross-repo/detached head, or a missing/null head.repo field — while still
letting legit same-repo PRs run the required gate with the token. The
workflow_dispatch and push-to-main clauses are unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
agent-reviewer-cr2 requested changes 2026-06-25 21:56:55 +00:00
Dismissed
agent-reviewer-cr2 left a comment
Member

REQUEST_CHANGES on head 857eb6caad.

The fork-gate fix in template-delivery-e2e.yml is correct: the positive allowlist github.event.pull_request.head.repo.full_name == github.repository fails closed for fork/cross-repo/null head cases. However, all seven core KMS fetches still use the old extractor shape print(json.load(sys.stdin)["secret"]["secretValue"]). If Infisical returns JSON null for secretValue, Python prints literal None, which passes [ -z "$CP_ADMIN_API_TOKEN" ] and can export/use CP_ADMIN_API_TOKEN=None. This is the same fail-open class we just blocked on #971. Please change the extractor to emit empty/non-zero for null or non-string values before export, then the existing non-empty gate will fail closed.

REQUEST_CHANGES on head 857eb6caada59aa8243b42d0250865aa57ad6d22. The fork-gate fix in template-delivery-e2e.yml is correct: the positive allowlist github.event.pull_request.head.repo.full_name == github.repository fails closed for fork/cross-repo/null head cases. However, all seven core KMS fetches still use the old extractor shape `print(json.load(sys.stdin)["secret"]["secretValue"])`. If Infisical returns JSON null for secretValue, Python prints literal `None`, which passes `[ -z "$CP_ADMIN_API_TOKEN" ]` and can export/use `CP_ADMIN_API_TOKEN=None`. This is the same fail-open class we just blocked on #971. Please change the extractor to emit empty/non-zero for null or non-string values before export, then the existing non-empty gate will fail closed.
agent-researcher requested changes 2026-06-25 21:58:51 +00:00
Dismissed
agent-researcher left a comment
Member

REQUEST_CHANGES on head 857eb6caad.

Correctness/security finding: the fork gate fix in template-delivery-e2e.yml is directionally correct — the executable condition now uses the fail-closed same-repo allowlist github.event.pull_request.head.repo.full_name == github.repository, so fork/null head.repo cases do not receive INFISICAL_CI_* secrets. However the KMS read helper introduced/kept in this PR still uses the old extractor form:

python3 -c 'import sys,json;print(json.load(sys.stdin)["secret"]["secretValue"])'

If Infisical returns JSON null for secretValue, Python prints the literal string None. That is non-empty, so the later [ -z "$CP_ADMIN_API_TOKEN" ] guard does not fail closed and the workflow can export/mask/use a bogus token. This is the same null-masking class fixed on #971 by emitting an empty string unless secretValue is a string.

Other axes: /shared/controlplane-admin path, add-mask-before-export, no secret deletion, and the new same-repo PR allowlist are otherwise in the right shape; full-paginated statuses show CI / all-required green, with review-gate/template pending/noise not changing the code finding. Please update the molecule-core KMS extractor(s) in this PR to the #971-safe pattern so null/non-string broken reads fail the non-empty check.

REQUEST_CHANGES on head 857eb6caada59aa8243b42d0250865aa57ad6d22. Correctness/security finding: the fork gate fix in template-delivery-e2e.yml is directionally correct — the executable condition now uses the fail-closed same-repo allowlist github.event.pull_request.head.repo.full_name == github.repository, so fork/null head.repo cases do not receive INFISICAL_CI_* secrets. However the KMS read helper introduced/kept in this PR still uses the old extractor form: python3 -c 'import sys,json;print(json.load(sys.stdin)["secret"]["secretValue"])' If Infisical returns JSON null for secretValue, Python prints the literal string None. That is non-empty, so the later [ -z "$CP_ADMIN_API_TOKEN" ] guard does not fail closed and the workflow can export/mask/use a bogus token. This is the same null-masking class fixed on #971 by emitting an empty string unless secretValue is a string. Other axes: /shared/controlplane-admin path, add-mask-before-export, no secret deletion, and the new same-repo PR allowlist are otherwise in the right shape; full-paginated statuses show CI / all-required green, with review-gate/template pending/noise not changing the code finding. Please update the molecule-core KMS extractor(s) in this PR to the #971-safe pattern so null/non-string broken reads fail the non-empty check.
hongming added 1 commit 2026-06-25 22:01:30 +00:00
ci(security): null-safe KMS secretValue extractor across all 7 fetch steps
CI / Python Lint & Test (pull_request) Successful in 7s
Block integration-tester contamination artifacts / Block staging-trigger / invalid manifest contamination (pull_request) Successful in 9s
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 9s
E2E Peer Visibility (literal MCP list_peers) / detect-changes (pull_request) Successful in 15s
Lint curl status-code capture / Scan workflows for curl status-capture pollution (pull_request) Successful in 7s
E2E API Smoke Test / detect-changes (pull_request) Successful in 20s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 14s
Lint forbidden tenant-env keys / Scan for repo-host token write into tenant workspace surface (pull_request) Successful in 7s
CI / Detect changes (pull_request) Successful in 23s
Lint forbidden tenant-env keys / Scan workspace_secrets writers for forbidden env keys (pull_request) Successful in 8s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 23s
lint-required-workflows-docker-host-pinned / Lint docker-host pin on docker-touching workflows (pull_request) Successful in 9s
E2E Chat / detect-changes (pull_request) Successful in 32s
Lint publish-runner timeout-minutes / Lint publish-runner timeout-minutes (pull_request) Successful in 14s
lint-no-coe-on-required / lint-no-coe-on-required (pull_request) Successful in 18s
E2E Peer Visibility (literal MCP list_peers) / E2E Peer Visibility (local) (pull_request) Has been skipped
lint-setup-go-cache / lint-setup-go-cache (pull_request) Successful in 15s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 16s
lint-continue-on-error-tracking / lint-continue-on-error-tracking (pull_request) Successful in 34s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 8s
sop-checklist / review-refire (pull_request_target) Has been skipped
E2E Peer Visibility (literal MCP list_peers) / E2E Peer Visibility (pull_request) Successful in 6s
Lint workflow YAML (Gitea-1.22.6-hostile shapes) / Lint workflow YAML for Gitea-1.22.6-hostile shapes (pull_request) Successful in 18s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 2s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 3s
Lint pre-flip continue-on-error / Verify continue-on-error flips have run-log proof (pull_request) Successful in 29s
sop-checklist / all-items-acked (pull_request) acked: 0/9 — missing: comprehensive-testing, local-postgres-e2e, staging-smoke, +6 — body-unfilled: comprehensive-testing, local-postgres-e2
sop-checklist / na-declarations (pull_request) N/A: (none)
CI / Shellcheck (E2E scripts) (pull_request) Successful in 2s
CI / Canvas (Next.js) (pull_request) Successful in 3s
CI / Platform (Go) (pull_request) Successful in 4s
sop-checklist / all-items-acked (pull_request_target) Successful in 9s
gate-check-v3 / gate-check (pull_request_target) Failing after 16s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 4s
CI / Canvas Deploy Status (pull_request) Successful in 2s
E2E Chat / E2E Chat (pull_request) Successful in 5s
PR Diff Guard / PR diff guard (pull_request) Successful in 22s
template-delivery-e2e / detect-changes (pull_request) Successful in 21s
CI / all-required (pull_request) Successful in 4s
Local Provision Lifecycle E2E / Local Provision Lifecycle E2E (stub) (pull_request) Successful in 40s
lint-required-context-exists-in-bp / lint-required-context-exists-in-bp (pull_request) Successful in 55s
reserved-path-review / reserved-path-review (pull_request_target) Approved via pull_request_review trigger
qa-review / approved (pull_request_target) Approved via pull_request_review trigger
reserved-path-review / reserved-path-review (pull_request_review) Successful in 10s
qa-review / approved (pull_request_review) Successful in 10s
security-review / approved (pull_request_target) Approved via pull_request_review trigger
security-review / approved (pull_request_review) Successful in 12s
Local Provision Lifecycle E2E / Local Provision Lifecycle E2E (real image + MiniMax LLM, advisory) (pull_request) Successful in 1m58s
audit-force-merge / audit (pull_request_target) Successful in 8s
template-delivery-e2e / Template-asset delivery (fresh seo-agent — config+prompts via asset channel, seo-all via plugin reconcile) (pull_request) Successful in 7m6s
E2E Staging SaaS (full lifecycle) / pr-validate (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / E2E Staging SaaS (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / Prune stale e2e DNS records (pull_request) Blocked by required conditions
E2E Staging SaaS (full lifecycle) / E2E Staging Platform Boot (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / E2E Staging Concierge user_tasks (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / E2E Staging Workspace Requests (core#2606) (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / E2E Staging Concierge Creates Workspace (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / E2E Staging Concierge (compile+skip) (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / E2E Staging Concierge Platform Agent (pull_request) Waiting to run
E2E Staging SaaS (full lifecycle) / E2E Staging Plugin Install Lifecycle (pull_request) Waiting to run
cecdbd8981
Replace print(json.load(...)["secret"]["secretValue"]) with a null-safe
extractor that emits an empty string for json-null / missing / non-string
secretValue, so the existing [ -z ] non-empty assertion fails CLOSED instead
of accepting the literal string 'None'. Same null-masking fail-open class
already fixed on molecule-controlplane #971 (fd25d9df).

Addresses CR2 REQUEST_CHANGES 14260. Fork-gate, ::add-mask::, and [ -z ]
assertions left untouched.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
agent-reviewer-cr2 approved these changes 2026-06-25 22:04:02 +00:00
agent-reviewer-cr2 left a comment
Member

Reviewed head cecdbd8981.

The prior blockers are resolved. The template-delivery KMS step now uses the positive same-repo allowlist (head.repo.full_name == github.repository), so untrusted fork PRs do not get the Infisical credential path. All seven CP_ADMIN_API_TOKEN KMS-fetch extractors now use the null-safe JSON idiom that emits an empty string unless secretValue is a string; that makes JSON null/non-string reads fail closed at the existing [ -z ] guard instead of exporting literal None. The old print(json.load(...)["secret"]["secretValue"]) pattern is gone from the PR diff.

5-axis review: correctness matches the wave-3 migration intent; robustness/fail-closed behavior is improved for broken Infisical reads and fork gating; security posture is clean with add-mask/export ordering retained and no secret deletion; performance impact is only the existing one-time workflow fetch; readability is acceptable and the comment documents why the fail-closed allowlist is used. Full-paginated statuses show CI/all-required and Secret scan success on this head, with remaining failures/pending contexts outside the code-review verdict path.

Verdict: APPROVED.

Reviewed head cecdbd89810e24644a54aa920a7f962ef219c5a9. The prior blockers are resolved. The template-delivery KMS step now uses the positive same-repo allowlist (`head.repo.full_name == github.repository`), so untrusted fork PRs do not get the Infisical credential path. All seven CP_ADMIN_API_TOKEN KMS-fetch extractors now use the null-safe JSON idiom that emits an empty string unless `secretValue` is a string; that makes JSON null/non-string reads fail closed at the existing `[ -z ]` guard instead of exporting literal `None`. The old `print(json.load(...)["secret"]["secretValue"])` pattern is gone from the PR diff. 5-axis review: correctness matches the wave-3 migration intent; robustness/fail-closed behavior is improved for broken Infisical reads and fork gating; security posture is clean with add-mask/export ordering retained and no secret deletion; performance impact is only the existing one-time workflow fetch; readability is acceptable and the comment documents why the fail-closed allowlist is used. Full-paginated statuses show CI/all-required and Secret scan success on this head, with remaining failures/pending contexts outside the code-review verdict path. Verdict: APPROVED.
agent-researcher approved these changes 2026-06-25 22:04:11 +00:00
agent-researcher left a comment
Member

APPROVED on head cecdbd8981.

Scope reviewed: wave-3 CP_ADMIN_API_TOKEN -> Infisical KMS migration across molecule-core workflows, including the template-delivery fork-gate fix.

My RC 14262 is resolved. I verified all seven KMS fetch helpers now use the #971-safe extractor form that emits an empty string unless secretValue is a JSON string, so json-null/non-string broken reads hit the following [ -z ] fail-closed guard instead of becoming literal None. The old print(json.load(...)["secret"]["secretValue"]) pattern count is zero.

5-axis review: correctness/robustness/security/performance/readability look clean. The seven workflows read /shared/controlplane-admin, call ::add-mask:: before exporting, assert non-empty before writing GITHUB_ENV, and do not delete the existing CP_ADMIN_API_TOKEN secret. The template-delivery trusted-ref gate remains fail-closed for fork/null PRs via github.event.pull_request.head.repo.full_name == github.repository; the remaining head.repo.fork == false text is explanatory comment only. Full-paginated statuses show CI / all-required and Secret scan green; reserved-path/review-gate reds are the expected pre-approval checks/noise.

APPROVED on head cecdbd89810e24644a54aa920a7f962ef219c5a9. Scope reviewed: wave-3 CP_ADMIN_API_TOKEN -> Infisical KMS migration across molecule-core workflows, including the template-delivery fork-gate fix. My RC 14262 is resolved. I verified all seven KMS fetch helpers now use the #971-safe extractor form that emits an empty string unless secretValue is a JSON string, so json-null/non-string broken reads hit the following [ -z ] fail-closed guard instead of becoming literal None. The old print(json.load(...)["secret"]["secretValue"]) pattern count is zero. 5-axis review: correctness/robustness/security/performance/readability look clean. The seven workflows read /shared/controlplane-admin, call ::add-mask:: before exporting, assert non-empty before writing GITHUB_ENV, and do not delete the existing CP_ADMIN_API_TOKEN secret. The template-delivery trusted-ref gate remains fail-closed for fork/null PRs via github.event.pull_request.head.repo.full_name == github.repository; the remaining head.repo.fork == false text is explanatory comment only. Full-paginated statuses show CI / all-required and Secret scan green; reserved-path/review-gate reds are the expected pre-approval checks/noise.
agent-reviewer-cr2 merged commit f08839974b into main 2026-06-25 22:06:46 +00:00
Sign in to join this conversation.
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#3274