Skip to content

fix(security): registry frontmatter parse failure erases publisher attribution — surface as typed rejection, don't suppress (#756 Cat 1) #791

Description

@ClintEastman02

Follow-up to #756 (Category 1 from @scottschreckengaust's triage on #756). Part of #756. Security-relevant — references the #664 SKILL.md frontmatter-injection review.

Problem

Three sites convert a parse failure into a valid-but-empty trust decision. A malformed/unparseable payload is silently treated as "empty", which for these sites means attacker-influenced input can erase attribution or downgrade a trust/screening decision.

Sites (current — after the #779 registry migration)

⚠️ Two of these were newly nosemgrep-suppressed during #779 rather than fixed — so the scanner reminder was removed while the fail-open behavior remained. That is precisely the #730 trap: "a suppression that quiets the scanner without removing the capability is worse than the finding, because it also removes the reminder."

  • cdk/src/handlers/shared/registry/agent-registry-client.ts:159parseSkillFrontmatter returns {} on yaml.load throw. Consumed by parseSkillPublisher, which recovers the publisher Cognito sub. Malformed YAML → publisher: undefined, indistinguishable from a record with no publisher. The docblock's injection defense ("a duplicate key is a YAML error") depends on that YAML error being surfaced, not swallowed into {}. Currently suppressed with -- invalid YAML becomes an unreadable payload, which does not address the attribution-erasure concern.
  • agent/src/registry/agent_registry_client.py:87 — the Python mirror, return {} on yaml.YAMLError, feeding _extract_runtime. A record whose runtime silently resolves to {} gets default runtime treatment rather than being rejected. Currently suppressed.
  • cdk/src/handlers/shared/orchestration-store.ts:344parsePreScreenedAttachments returns [] when pre_screened_attachments_json is unparseable. [] means both "no attachments were pre-screened" AND "screening state is unknown" — and children are released either way. Load-bearing for a content-screening decision. (Still active — not yet suppressed.)

Fix

Return a discriminated result ({ ok: true, value } | { ok: false, reason }) or raise a typed RegistryRecordMalformedError / equivalent, and reject the record rather than proceeding with an empty trust decision. Remove the two inline suppressions once the behavior is fixed at source. Keep the TS and Python paths in parity.

Line numbers verified against the PR #788 branch. Credit: triage by @scottschreckengaust on #756; migration context #779.

Metadata

Metadata

Assignees

No one assigned

    Labels

    P1medium priorityregistryAgent asset registry: capabilities, skills, plugins, MCP servers, blueprintssecurityCedar/HITL, IAM least-privilege, secrets, PII/DLP, guardrails, supply-chain/CVE

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions