feat: enhanced structural validation for validate command#209
feat: enhanced structural validation for validate command#209bryan-anthropic wants to merge 1 commit intomainfrom
Conversation
Add three pre-pack validators to catch common runtime failures early:
- validateEntryPoint: checks file exists, extension matches server type,
binary has executable bit (Unix)
- validateCommandVariables: scans mcp_config for unrecognized ${...}
variables against the runtime allowlist
- validateSensitiveFiles: warns about credentials.json, *.pem, *.key,
.ssh/, etc. that would be bundled
validateManifest() now accepts optional { projectDir } to separate the
manifest directory from the source file directory.
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
|
@claude can you review this |
There was a problem hiding this comment.
Review by Claude Code
Looks good — three practical pre-pack validators that catch real issues early (missing entry points, invalid variables, sensitive file leaks).
A few observations:
-
Clean API design — the
ValidationResultinterface and themanifestDirvsprojectDirseparation are well thought out. Pack passingprojectDirto handle external manifests is a nice touch. -
Sensitive files as warnings-only — correct call. A
.pemcould be a legitimate TLS cert; erroring would be too opinionated. -
Test coverage — 11 new integration tests covering all three validators, plus existing fixtures updated for entry_point checks. Solid.
Minor nit: the silent catch {} in validateSensitiveFiles could mask unexpected errors from getAllFilesWithCount, but the comment explains the rationale (pack will fail with a clearer error downstream). Fine as-is.
2 participants
Summary
mcpb validatethat catch common runtime failures early:mcp_configfor unrecognized${...}patterns (e.g.${BUNDLE_ROOT}which silently fails at runtime)credentials.json,*.pem,*.key,.ssh/etc. that would be bundledvalidateManifest()accepts optional{ projectDir }to support external manifest pathsCloses #57
Test plan
yarn build— cleanyarn lint— cleanyarn test— 229 pass (218 existing + 11 new)${BUNDLE_ROOT}→ fix to${__dirname}→ pack → MCP initialize succeedscredentials.jsonin subdir → warning (exits 0)🤖 Generated with Claude Code