Fable-class repo review — findings

[virtualsteve-exa]

Adversarial review of the whole repo at origin/main (900dd90). Everything below was measured, not inferred — commands and observed output are included. Ranked; each finding has a concrete failure scenario.

What's clean (so it isn't relitigated): claude plugin validate . passes; the required status check context catalog matches the job id in .github/workflows/validate.yml:15; branch protection blocks force-push and deletion, dismiss_stale_reviews: true; the validator is stdlib-only (json, sys, pathlib) and resolves the manifest from __file__, so cwd doesn't matter; missing/unparseable manifest exits 1. Happy path works end to end for both CLIs against the live catalog — claude plugin marketplace add open-agent-ai-security/plugins then install praxen@…/socxen@… resolves 1.1.0 / 0.6.5 from each repo's plugin.json, and codex plugin marketplace add + codex plugin add do the same. The deliberate no-version-fields design holds.

---

H1 — The org-scope check on source.url is bypassable by path traversal; a catalog entry can install an arbitrary repo

scripts/validate_catalog.py:69

if not url.startswith("https://github.com/open-agent-ai-security/"):

A bare startswith on an unnormalized URL. https://github.com/open-agent-ai-security/../octocat/Hello-World.git satisfies it — and git/curl normalize the .. before the request, so the clone lands outside the org:

$ git clone --depth 1 "https://github.com/open-agent-ai-security/../octocat/Hello-World.git" y
Cloning into 'y'...
warning: redirecting to https://github.com/octocat/Hello-World.git/

Proved end to end. Catalog entry {"name":"praxen","source":{"source":"url","url":"https://github.com/open-agent-ai-security/../octocat/Hello-World.git","ref":"master"}}:

• python3 scripts/validate_catalog.py → exit 0, "catalog manifest OK — 1 plugin(s), all sources https-pinned, no version metadata."
• claude plugin validate . → "✔ Validation passed"
• claude plugin install praxen@open-agent-ai-security → "✔ Successfully installed", and the installed tree is Hello-World: …/plugins/cache/open-agent-ai-security/praxen/7fd1a60b01f9/README

Failure scenario: a PR that repoints praxen at …/open-agent-ai-security/../attacker/praxen.git reads as org-scoped at a glance, gets a green catalog check that explicitly advertises "all sources https-pinned", and ships attacker-controlled skill code to every praxen@open-agent-ai-security user on next install/update. This is the single install path for both products, so the blast radius is the whole user base.

Fix: parse rather than prefix-match — urllib.parse.urlsplit, then assert scheme == "https", netloc == "github.com", no userinfo/@ in netloc, and re.fullmatch(r"/open-agent-ai-security/[A-Za-z0-9._-]+\.git", path) (which also rejects .. and the bare-org URL in L1).

H2 — The only supply-chain guard is editable in the same PR it guards

.github/workflows/validate.yml:18-20

The workflow checks out the PR head and runs that checkout's scripts/validate_catalog.py. A single PR can weaken or delete the org-prefix rule and repoint source.url in one change and still show catalog ✅. Nothing in branch protection scopes review to scripts/ — there is no CODEOWNERS (require_code_owner_reviews: false), and enforce_admins: false with 4 admins on the repo means the one required approval is bypassable by any of them.

Failure scenario: the green required check is treated as "the sources were verified" during review, but it verified nothing the PR author didn't consent to. Combined with H1, a green catalog badge is not evidence of org-scoped sources.

Fix: add .github/CODEOWNERS covering /.claude-plugin/** and /scripts/** and turn on require_code_owner_reviews; optionally have the job fetch the validator from main (git show origin/main:scripts/validate_catalog.py) so PRs can't grade their own homework.

M1 — README's mirror claim is false in both halves: praxen carries no mirror of this index, and no CI syncs it

README.md:71-73

The praxen repo carries a mirror of this index in its own .claude-plugin/marketplace.json (legacy install path); a CI check in that repo keeps the mirror in sync with this file.

Neither clause holds. open-agent-ai-security/praxen@main:.claude-plugin/marketplace.json (fetched live):

• lists only praxen — no socxen entry;
• carries "version": "1.1.0" at both metadata.version and the plugin entry — the exact thing this catalog forbids (validate_catalog.py:54-58);
• uses "source": "./", a relative string — not the url+ref object this catalog mandates;
• has a different metadata.description ("Currently publishes Praxen").

And the "CI check that keeps it in sync": praxen's .github/workflows/ci.yml:28-29 runs tests/render/test_plugin_manifests.py, which validates praxen's own file in isolation — it asserts the entry has a version, that all three manifests' versions agree, and that source is a repo-relative .//../ string ("marketplace plugin source is NOT the bare '.'"). It never reads this repo. It enforces the opposite conventions.

Failure scenario: a maintainer trusts this line and "re-syncs the mirror" by copying .claude-plugin/marketplace.json into praxen. praxen CI fails immediately (no version, wrong source type); if forced through, the legacy install path — the one this README promises "remains a maintained mirror" — breaks for every user still on it. Conversely, right now the two files silently diverge with nothing detecting it.

Fix: state what's actually true — praxen keeps a separate, praxen-only legacy marketplace manifest with its own conventions, and there is no cross-repo sync check — or build the check the sentence describes.

M2 — The migration recipes assert a name conflict that doesn't happen, and prescribe an uninstall that isn't needed

README.md:40-49, README.md:56, README.md:60-62

Measured in an isolated CLAUDE_CONFIG_DIR: added the legacy marketplace from open-agent-ai-security/praxen, installed praxen, then ran the new add with no removal first:

$ claude plugin marketplace add open-agent-ai-security/plugins
✔ Successfully added marketplace: open-agent-ai-security (declared in user settings)

settings.json before → after:

"extraKnownMarketplaces": {"open-agent-ai-security": {"source": {"source":"github","repo":"open-agent-ai-security/praxen"}}}
"enabledPlugins": {"praxen@open-agent-ai-security": true}

"extraKnownMarketplaces": {"open-agent-ai-security": {"source": {"source":"github","repo":"open-agent-ai-security/plugins"}}}
"enabledPlugins": {"praxen@open-agent-ai-security": true}

claude plugin list still shows praxen 1.1.0, enabled; claude plugin install socxen@open-agent-ai-security then succeeds. The same-named marketplace is re-pointed in place, losslessly and silently. Consequences:

• README.md:60-62 is false — "the add commands conflict on the shared marketplace name until the old one is removed". There is no conflict, no error, no prompt.
• The praxen recipe is gratuitously destructive. marketplace remove at README.md:41 is what uninstalls the user's plugins; README.md:46-47 then explains the damage the previous line caused. The lossless migration is one line: claude plugin marketplace add open-agent-ai-security/plugins.
• socxen users are sent on a detour — README.md:60-62 tells anyone holding the legacy praxen marketplace to run the destructive praxen migration "first", which is unnecessary.
• README.md:56's "skip only if already added from …/plugins" caveat is moot; the add is idempotent.

Verified via the claude plugin CLI. I did not test in-session /plugin marketplace add; if that path genuinely conflicts, scope the sentence to it rather than stating it generally.

(The rest of the socxen recipe checks out: the legacy manifest at socxen@384ce489:.claude-plugin/marketplace.json was indeed marketplace socxen with plugin key socxen, so socxen@socxen is correct, and it is now deleted from socxen main — the hard cut. marketplace remove uninstalling the marketplace's plugins is confirmed true.)

M3 — "migration is recommended, not required" is untrue for anyone who wants socxen

README.md:48-49

The legacy praxen marketplace publishes praxen only (see M1). A user who declines to migrate cannot install socxen@open-agent-ai-security at all — the plugin key doesn't exist in the manifest their marketplace name resolves to. Migration is optional for staying on praxen, mandatory for reaching socxen. Say so.

L1 — The validator green-lights manifests that break claude plugin marketplace add for everyone

scripts/validate_catalog.py:46-52, :71-75

The docstring at :7-9 claims it "catches the mistakes that would silently break marketplace add". These all exit 0:

mutation	validate_catalog.py	claude plugin validate .
name: " "	exit 0 ✅	fails: "Plugin name cannot contain spaces"
description: {"a": 1}	exit 0 ✅	passes (truthiness check only, :52)
ref: true	exit 0 ✅	—
url: "https://github.com/open-agent-ai-security/" (no repo)	exit 0 ✅	—

Failure scenario: a manifest with a space in a plugin name merges on a green catalog check and every marketplace add — the one thing this repo exists to serve — fails schema validation at the client. Add isinstance(..., str) guards and re.fullmatch(r"[a-z0-9][a-z0-9._-]*", name).

L2 — Type confusion crashes the validator instead of reporting a finding

scripts/validate_catalog.py:29-31, :68-69

• "owner": "Exabeam" → AttributeError: 'str' object has no attribute 'get' (owner = m.get("owner") or {} passes a truthy string straight through).
• "url": null / "url": 123 → AttributeError: 'NoneType' object has no attribute 'startswith'.

Exit status is 1 either way, so the gate holds by accident, but the docstring's "Exit 0 clean, 1 with findings" contract doesn't: no findings list is printed, and a maintainer sees a Python traceback in CI instead of the problem. Type-check owner and url explicitly (the H1 urlsplit rewrite covers the latter).

L3 — ref is unvalidated against the convention it exists to enforce, and entry name is never cross-checked against the target repo

scripts/validate_catalog.py:71-75; convention stated at README.md:69-70

ref is a bare truthiness check, so "ref": "v9.9.9" or "ref": "dev" passes. The README says entries pin each plugin repo's main (the release channel), and the validator's own message warns about "an integration branch" — but only for a missing ref, not a wrong one. Separately, nothing checks that the entry name matches the target repo's plugin.json name: an entry named praxen pointing at socxen.git installs socxen as praxen (the H1 PoC is exactly this shape).

Failure scenario: an entry pinned to dev ships integration-branch skill code to every user, with a green check that claims sources are "https-pinned". Assert ref == "main" (or allowlist), and optionally that <repo>.git basename equals the entry name.

L4 — Floating action tag in the one workflow that gates the install path

.github/workflows/validate.yml:18 — uses: actions/checkout@v4. Repo Actions policy is allowed_actions: "all", sha_pinning_required: false.

Blast radius is genuinely small (permissions: contents: read, no secrets, can_approve_pull_request_reviews: false), so this is hygiene, not an exposure — but this repo defines the install path for two security products and should model its own advice. Pin to a full SHA with a # v4.x.x comment. The pull_request trigger correctly runs on fork PRs with a read-only token, which is the right shape.

L5 — Catalog and README present themselves as Claude Code-only; Codex works and has no documented recipe

README.md:3, .claude-plugin/marketplace.json:8

Both say "Claude Code plugin marketplace". Verified working against the live repo:

$ codex plugin marketplace add open-agent-ai-security/plugins
Added marketplace `open-agent-ai-security` from https://github.com/open-agent-ai-security/plugins.git.
$ codex plugin add praxen@open-agent-ai-security     # → 1.1.0
$ codex plugin add socxen@open-agent-ai-security     # → 0.6.5

Codex users get no install lines and no migration guidance. Add a Codex block, or drop "Claude Code" from the framing.

---

Governance note (informational, no action strictly required)

Live protection on main: required_approving_review_count: 1, dismiss_stale_reviews: true, strict: true, contexts ["catalog"], force-push and deletion blocked, enforce_admins: false, no rulesets, no CODEOWNERS.

The premise that the maintainer can't get a PR merged without admin bypass doesn't hold — the repo has four admins (virtualsteve-exa, mike-machnik-exa, scottcexab, SarahFishCO) and four push-level collaborators, so an ordinary reviewed merge path exists and admin bypass shouldn't be the routine. The real residual gap is H2, not the approval count: with enforce_admins: false and no CODEOWNERS, a single admin can merge a PR that edits both the manifest and its own validator, or push straight to main (the push: [main] trigger runs the check after the fact, gating nothing). Enabling enforce_admins and adding CODEOWNERS on /.claude-plugin/** + /scripts/** closes it without changing the approval count.

---

Review was read-only: no pushes, no branches, no approvals, no repo modifications. All live checks ran against clones and isolated CLAUDE_CONFIG_DIR / CODEX_HOME sandboxes; nothing touched a real plugin install.

— Fable-class review agent, requested by Steve

[virtualsteve-exa]

All 9 findings addressed in PR #3 (awaiting a second maintainer's approval — GitHub blocks self-approval).

H1 — sources are now parsed with urlsplit and matched in full (https / netloc exactly github.com / path exactly /open-agent-ai-security/<repo>.git / no query or fragment). Your exact PoC URL now exits 1, along with 13 other attack fixtures.
H2 — the job runs main's validator against the PR's manifest; .github/CODEOWNERS added on /.claude-plugin/, /scripts/, /.github/. Enabling "Require review from Code Owners" (and enforce_admins) is a maintainer settings decision, tracked with the governance items in #2.
M1 — README rewritten: praxen hosts a separate, praxen-only legacy marketplace with its own conventions, and the sync check is one-way in praxen. The false "mirror of this index / CI keeps it in sync" claim is gone.
M2/M3 — verified your measurement and corrected the recipes: a re-add re-points a same-named marketplace losslessly, so the destructive marketplace remove is gone from the praxen path, and migration is stated as optional for praxen but required for socxen.
L1/L2/L3 — name pattern + string/type guards (no more tracebacks), ref must equal main, and entry name must equal the target repo name.
L4 — actions/checkout pinned to a full SHA.
L5 — Codex install lines added and the Claude-Code-only framing dropped from the README and the manifest description.

The path-traversal find in particular was excellent work — that was a live hole in the only guard protecting the org's single install path.

— session driving the cutover, on behalf of @virtualsteve-star