Repository navigation
Resolve Claude directory privacy and credential warnings - #16
Conversation
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline finding, I also checked two other angles: the --secret-stdin usage examples in verify_signature.py's docstring and the mirrored SKILL.md files never show the required stdin redirection (e.g. echo secret | python3 ...), so copy-pasting them as written would hang waiting on a TTY rather than leak anything; and the new "ambient credential read" regex in scripts/validate.py covers os.environ/os.getenv/process.env/os.Getenv/etc. but not bare PHP getenv(...) or System.getenv(...) in Java, so those idioms would slip past the gate in public-facing examples.
Extended reasoning...
The diff hardens credential handling in the Sent plugin: verify_signature.py switches from an env-var flag to a stdin-only secret flag, SKILL.md/reference docs forbid ambient credential reads, and validate.py gains a regex gate plus a new manifest-field cross-check against the canonical marketplace listing. The confirmed inline finding is a KeyError risk from dict-subscripting a possibly-missing listing field in validate_adapters, inconsistent with the rest of the file's defensive .get() usage. Decided defer because a real finding is being posted inline and the change touches security-sensitive validation/credential-handling logic that still warrants human attention, not because of the two ruled-out items themselves.
| listing = load_json(MARKETPLACE_CONTENT) | ||
| for field, source in (("documentationUrl", "website_url"), ("supportUrl", "support_url"), | ||
| ("privacyPolicyUrl", "privacy_policy_url"), ("termsOfServiceUrl", "terms_of_service_url")): |
There was a problem hiding this comment.
🟡 (optional) Validator maintainers get a crashing traceback instead of a validation report if the canonical marketplace.json ever drops a listing URL field, unlike every other check in this file. listing[source] at validate.py:974 uses dict subscripting (KeyError on a missing key) while the rest of validate_adapters and validate_marketplace_content use .get() plus a soft validation.check. A missing website_url/support_url/privacy_policy_url/terms_of_service_url only soft-fails earlier (validate_marketplace_content, line 516: "canonical marketplace content fields drifted") but then crashes validate_adapters before the consolidated report prints. Fix: use listing.get(source) and report a normal validation failure instead of letting a missing key raise.
Why this was flagged
Trigger: adapter-sources/shared/marketplace.json loses one of website_url/support_url/privacy_policy_url/terms_of_service_url (e.g. an editing mistake), reached via the validate() pipeline which runs validate_marketplace_content (line 1076) before validate_adapters (line 1085). validate_marketplace_content only records a soft error ("canonical marketplace content fields drifted", line 516) and does not stop execution. validate_adapters then does claude_manifest.get(field) == listing[source] at line 974, and listing[source] raises KeyError since listing is a plain dict missing that key. On base branch there is no such check at all, so no crash; after this change CI aborts with an unhandled traceback instead of a clean validation failure list, and every other check elsewhere in validate_adapters that would have run after line 974 is skipped.
Verification: scripts/validate.py:975 uses a bare dict subscript, listing[source], on listing loaded at 972 from MARKETPLACE_CONTENT. validation.check (140-142) only appends to errors and never halts, so a missing website_url/support_url/privacy_policy_url/terms_of_service_url field lets validate_marketplace_content record the soft error "canonical marketplace content fields drifted" (line 516) and continue.
Claude directory validation flagged missing privacy metadata and ambient credential reads in the distributed utility/examples. Sent v0.2.1 supplies documentationUrl, supportUrl, privacyPolicyUrl, and termsOfServiceUrl in the Claude manifest using the shared listing source.
The local webhook signature utility now accepts an explicitly supplied signing secret on standard input with --secret-stdin and ignores installer environment credentials. SDK and receiver examples use explicit application-owned credentials or injected signing secrets. The remote Sent MCP configuration continues to use client-managed OAuth.
Adds credential-input regression tests and rejection gates for ambient credential reads and a missing Claude privacy field. Updates the pinned Claude validator to 2.1.287 so CI recognizes directory listing metadata, and regenerates all adapters.
Validation: 51 regression tests, strict Claude marketplace/plugin validation, generated parity, webhook self-test, and whitespace checks pass locally. CI runs the complete release rejection suite. Claude directory ingestion/security scanning must rerun against v0.2.1; local validation does not establish directory acceptance.
The utility's --secret-env option has been replaced by --secret-stdin to remove ambient credential access.