Found while verifying codex-security scan candidates. All four confirmed by reading the code; two were reproduced by executing the cited functions verbatim.
Shared trust boundary, stated up front: every item here requires control of adcontextprotocol.org, push access to adcontextprotocol/adcp, or HTTPS MITM. No untrusted third party can trigger them. That is why these are Medium rather than High — but each converts "I control the schema bundle" into "I execute code on developer machines and in privileged CI jobs," which is a meaningful escalation.
1. sync-agent-roles.yml executes a script it just downloaded (candidate-8eb3c32656ac1129)
.github/workflows/sync-agent-roles.yml:31-42 fetches refs/heads/main.tar.gze from adcontextprotocol/adcp unpinned, copies scripts/import-claude-agents.mjs out of it at :38, and runs it with Node at :42 — in a job holding contents: write + pull-requests: write. actions/checkout@v6 at :21 defaults to persist-credentials: true, so the job token is in .git/config where the downloaded script can read it.
The fix is free: the script is already committed at scripts/import-claude-agents.mjs, and validate-agent-roles-sync.yml proves it runs standalone. Regenerate with the HEAD copy before overwriting it — the upstream change then rides in the PR and only executes after a human approves it. Add persist-credentials: false too.
2. generate-manifest-derived.ts interpolates manifest keys as TypeScript syntax (candidate-52775008039b6fc4)
scripts/generate-manifest-derived.ts has two unquoted interpolation sites, and loadManifest (:86) validates only that three sections exist:
:159 — ` ${code}: {\n description: …` emits a manifest key as a bare object property name.:168 + :248 — `export const ${constName} = [` where protocolNameToConst only does .replace(/-/g,'_').toUpperCase(), so ;, =, and spaces survive into identifier position.
Both breakout shapes were compiled with the repo's own tsc --strict and parse clean. A working key for the first site closes the object literal, runs arbitrary statements, and reopens so the trailing } as const satisfies Record<…> still balances.
This is the one that matters most, because output is src/lib/types/manifest.generated.ts → imported by src/lib/types/error-codes.ts → bundled by build:lib → shipped. Injected statements execute at import time in every @adcp/sdk consumer process. It is the only item here where upstream-bundle control becomes code injection rather than data injection.
Review posture makes it worse: schema-sync.yml commits the regenerated file into an auto-PR, and CLAUDE.md explicitly tells reviewers not to read generated files. ci.yml:54 does fail on drift, so it's detected — but detection lands in a PR nobody is expected to read.
The fix pattern is already in the same file: the specialism block at :174 uses JSON.stringify(id) for its key. Do that at :159, and regex-validate error-code and protocol keys in loadManifest.
3. refToLocalPath writes outside the schema cache (candidate-8ea8e922cc44de82)
scripts/sync-schemas.ts:528-536. Executed verbatim:
/schemas/3.1.8/../../../../.npmrc -> /.npmrc
/schemas/3.1.8/../../.git/hooks/pre-commit -> <repo>/schemas/.git/hooks/pre-commit
The .. normalizes away in the fetch URL (WHATWG parsing), so the attacker serves the payload at the normalized path — which they can, since they control the origin. Written content is always JSON, but that is not a constraint: package.json is JSON, and a "scripts": {"prepare": …} overwrite is code execution on the next npm install.
Gated behind the tarball endpoint 404ing (:371-375), though the same origin controls that too.
Fix: path.resolve + startsWith(cacheDir + path.sep) containment throw.
scripts/generate-3-1-beta-types.ts:63-76 (candidate-3e0df3f4af77ca14) has the same shape on the read side into readFileSync. Strictly lower severity — 3.1-beta appears in zero workflow files, so it needs a human running npm run generate-types:3.1-beta — but fix both together. While in that file: the JSDoc at :21-22 claims "CI's 'validate generated files in sync' gate runs both"; ci.yml:47-59 runs generate-types and generate-types:v2.5 only, so the stated guarantee doesn't exist.
4. Cosign verification is present only where it cannot protect anything (candidate-b4fe9a2c37f81153)
sync-schemas.ts:369 derives shaUrl as tgzUrl + '.sha256' — digest and artifact share an origin, so it's integrity, never authenticity. verifyCosignSignature skips at :194 (latest), :207 (404 sidecar), and :216-224 (cosign binary absent → warn and continue).
Which callers actually have cosign is the part the scan missed:
| Workflow |
cosign installed? |
What it holds |
ci.yml:36 |
yes (SHA-pinned installer) |
can only fail a build |
schema-sync.yml |
no |
contents: write, opens the schema-bump PR |
release.yml |
no |
npm OIDC publish + IPR App token |
Signature verification is present exactly where it can't protect an artifact and absent in both places that produce one. Fix: install cosign in both, and fail closed via an ADCP_REQUIRE_SIGNATURE=1 style gate rather than warn-and-continue.
Correcting one sub-claim: the latest skip is currently unreachable (ADCP_VERSION is pinned to a release; no caller passes latest) — latent, not live. No preinstall/postinstall runs any of this, so npm consumers are unaffected; dev-and-CI only.
Related, lower priority
ai-review.yml:99 passes raw SECRETARIAT_APP_PRIVATE_KEY even though :42-45 already minted a scoped installation token in the same job. Pass steps.app-token.outputs.token instead — with the caveat that the Ladon composite may need the key to mint tokens with different permission sets, worth checking first.ipr-agreement.yml:20 calls adcontextprotocol/adcp/.github/workflows/ipr-check-callable.yml@main and passes IPR_APP_PRIVATE_KEY — a raw, long-lived, org-wide App private key crossing a repo boundary, where release.yml:37-41 already demonstrates the scoped-installation-token pattern. SHA-pinning would defeat the purpose (the IPR ledger must track upstream), so this is really a governance question: whether the write-access sets for adcp and adcp-client are identical. If the spec repo has broader write access, this is a one-way trust grant that should be explicit.- Not a finding, recorded to close it out: the "unpinned action" framing does not hold as a repo-wide criticism. Exactly 1 of ~28
uses: is SHA-pinned, so there is no convention being violated, and the flagged action is same-org. Three genuinely third-party actions on mutable major tags receive higher-value secrets (peter-evans/slash-command-dispatch@v5 and peter-evans/create-or-update-comment@v5 get a repo-scope PAT; changesets/action@v1 gets an App token in a job with npm publish rights). If pinning is worth doing, start there, not with ladon/review.
Found while verifying
codex-securityscan candidates. All four confirmed by reading the code; two were reproduced by executing the cited functions verbatim.Shared trust boundary, stated up front: every item here requires control of
adcontextprotocol.org, push access toadcontextprotocol/adcp, or HTTPS MITM. No untrusted third party can trigger them. That is why these are Medium rather than High — but each converts "I control the schema bundle" into "I execute code on developer machines and in privileged CI jobs," which is a meaningful escalation.1.
sync-agent-roles.ymlexecutes a script it just downloaded (candidate-8eb3c32656ac1129).github/workflows/sync-agent-roles.yml:31-42fetchesrefs/heads/main.tar.gzefromadcontextprotocol/adcpunpinned, copiesscripts/import-claude-agents.mjsout of it at:38, and runs it with Node at:42— in a job holdingcontents: write+pull-requests: write.actions/checkout@v6at:21defaults topersist-credentials: true, so the job token is in.git/configwhere the downloaded script can read it.The fix is free: the script is already committed at
scripts/import-claude-agents.mjs, andvalidate-agent-roles-sync.ymlproves it runs standalone. Regenerate with the HEAD copy before overwriting it — the upstream change then rides in the PR and only executes after a human approves it. Addpersist-credentials: falsetoo.2.
generate-manifest-derived.tsinterpolates manifest keys as TypeScript syntax (candidate-52775008039b6fc4)scripts/generate-manifest-derived.tshas two unquoted interpolation sites, andloadManifest(:86) validates only that three sections exist::159—` ${code}: {\n description: …`emits a manifest key as a bare object property name.:168+:248—`export const ${constName} = [`whereprotocolNameToConstonly does.replace(/-/g,'_').toUpperCase(), so;,=, and spaces survive into identifier position.Both breakout shapes were compiled with the repo's own
tsc --strictand parse clean. A working key for the first site closes the object literal, runs arbitrary statements, and reopens so the trailing} as const satisfies Record<…>still balances.This is the one that matters most, because output is
src/lib/types/manifest.generated.ts→ imported bysrc/lib/types/error-codes.ts→ bundled bybuild:lib→ shipped. Injected statements execute at import time in every@adcp/sdkconsumer process. It is the only item here where upstream-bundle control becomes code injection rather than data injection.Review posture makes it worse:
schema-sync.ymlcommits the regenerated file into an auto-PR, andCLAUDE.mdexplicitly tells reviewers not to read generated files.ci.yml:54does fail on drift, so it's detected — but detection lands in a PR nobody is expected to read.The fix pattern is already in the same file: the specialism block at
:174usesJSON.stringify(id)for its key. Do that at:159, and regex-validate error-code and protocol keys inloadManifest.3.
refToLocalPathwrites outside the schema cache (candidate-8ea8e922cc44de82)scripts/sync-schemas.ts:528-536. Executed verbatim:The
..normalizes away in thefetchURL (WHATWG parsing), so the attacker serves the payload at the normalized path — which they can, since they control the origin. Written content is always JSON, but that is not a constraint:package.jsonis JSON, and a"scripts": {"prepare": …}overwrite is code execution on the nextnpm install.Gated behind the tarball endpoint 404ing (
:371-375), though the same origin controls that too.Fix:
path.resolve+startsWith(cacheDir + path.sep)containment throw.scripts/generate-3-1-beta-types.ts:63-76(candidate-3e0df3f4af77ca14) has the same shape on the read side intoreadFileSync. Strictly lower severity —3.1-betaappears in zero workflow files, so it needs a human runningnpm run generate-types:3.1-beta— but fix both together. While in that file: the JSDoc at:21-22claims "CI's 'validate generated files in sync' gate runs both";ci.yml:47-59runsgenerate-typesandgenerate-types:v2.5only, so the stated guarantee doesn't exist.4. Cosign verification is present only where it cannot protect anything (
candidate-b4fe9a2c37f81153)sync-schemas.ts:369derivesshaUrlastgzUrl + '.sha256'— digest and artifact share an origin, so it's integrity, never authenticity.verifyCosignSignatureskips at:194(latest),:207(404 sidecar), and:216-224(cosign binary absent → warn and continue).Which callers actually have cosign is the part the scan missed:
ci.yml:36schema-sync.ymlcontents: write, opens the schema-bump PRrelease.ymlSignature verification is present exactly where it can't protect an artifact and absent in both places that produce one. Fix: install cosign in both, and fail closed via an
ADCP_REQUIRE_SIGNATURE=1style gate rather than warn-and-continue.Correcting one sub-claim: the
latestskip is currently unreachable (ADCP_VERSIONis pinned to a release; no caller passeslatest) — latent, not live. Nopreinstall/postinstallruns any of this, so npm consumers are unaffected; dev-and-CI only.Related, lower priority
ai-review.yml:99passes rawSECRETARIAT_APP_PRIVATE_KEYeven though:42-45already minted a scoped installation token in the same job. Passsteps.app-token.outputs.tokeninstead — with the caveat that the Ladon composite may need the key to mint tokens with different permission sets, worth checking first.ipr-agreement.yml:20callsadcontextprotocol/adcp/.github/workflows/ipr-check-callable.yml@mainand passesIPR_APP_PRIVATE_KEY— a raw, long-lived, org-wide App private key crossing a repo boundary, whererelease.yml:37-41already demonstrates the scoped-installation-token pattern. SHA-pinning would defeat the purpose (the IPR ledger must track upstream), so this is really a governance question: whether the write-access sets foradcpandadcp-clientare identical. If the spec repo has broader write access, this is a one-way trust grant that should be explicit.uses:is SHA-pinned, so there is no convention being violated, and the flagged action is same-org. Three genuinely third-party actions on mutable major tags receive higher-value secrets (peter-evans/slash-command-dispatch@v5andpeter-evans/create-or-update-comment@v5get arepo-scope PAT;changesets/action@v1gets an App token in a job with npm publish rights). If pinning is worth doing, start there, not withladon/review.