Sr 04

[rachellerathbone]

What

Brief description of changes

Why

Why this change was needed

How

Brief technical approach

Testing

How to verify the changes

[github-actions]

multicorn-ops review

Persona	Role	Primary	Status	Summary
Jordan	Security Auditor	yes	Concern	API key handling via manifest substitution is reasonable but several security gaps need attention before regulated-environment deployment.
Priya	Open Source Contributor	yes	Concern	The extension feature is well-documented in the README but the new source files (src/extension/server.ts, src/extension/restore.ts) are entirely absent from the diff, making the PR impossible to fully review or test.
Marcus	Design-Conscious Developer	no	Passed	No UI changes in this diff; all additions are CLI, manifest, and documentation.
Sarah	Non-Technical Decision-Maker	no	Concern	The README additions are mostly clear but contain developer jargon and an unfinished TODO that would undermine trust if a non-technical evaluator reads it.
The Team	Acquisition Due Diligence	yes	Concern	The PR adds significant surface area (MCP proxy, child process spawning, config mutation, new CLI) with the core implementation files absent from the diff, making architecture and test coverage assessment impossible.
Alex	Accessibility Advocate	no	Passed	No UI or HTML changes in this diff; accessibility cannot be assessed but is not regressed.
Yuki	International User	yes	Concern	Most copy is clear but a few technical terms and an incomplete TODO appear in user-facing text that would confuse non-native English readers.

Concerns

Jordan (Security Auditor)

• manifest.json:31 - MULTICORN_BASE_URL is hardcoded in the manifest with no way for enterprise customers to point to an on-premises or regional endpoint, creating a forced data-egress path that may violate data residency requirements.
• manifest.json:29 - The manifest substitutes the API key into an env var (MULTICORN_API_KEY). If the extension process is ever inspected via /proc//environ or equivalent on Linux, the key is exposed in plaintext. Confirm Claude Desktop's process isolation guarantees before shipping.
• docs/desktop-extension-research.md:25 - Research doc explicitly notes 'Sensitive values should not be written to project JSON on disk by the extension itself' but no code in this diff enforces or tests that invariant — there is no audit of what the extension writes to disk at runtime.
• src/extension/restore.js - File not visible in this diff. The restore command overwrites claude_desktop_config.json from a backup stored at ~/.multicorn/extension-backup.json. If that backup path is world-readable, or if an attacker can plant a crafted backup, this becomes an arbitrary config injection vector. Permissions on ~/.multicorn/ must be enforced (0700) and validated at read time.
• bin/multicorn-shield.ts:14 - Error messages on the restore path surface the raw Error.message directly to stderr. If the underlying implementation throws errors containing file paths, partial config content, or API responses, those internals leak to any process capturing stderr.
• docs/desktop-extension-research.md:14 - The proxy approach spawns child MCP server processes using stdio. There is no mention of sanitising or validating the command/args read from claude_desktop_config.json before exec. A maliciously crafted config entry could achieve command injection through the extension's own privilege context.
• README.md:67 - The restore flow says 'overwrites mcpServers in your config with the last backup' with no integrity check (hash, signature) on the backup file. Tampering with ~/.multicorn/extension-backup.json is not detected.

Priya (Open Source Contributor)

• src/extension/server.ts - The file is imported by bin/shield-extension.ts but is not present in this diff. A contributor cannot understand, run, or test the core extension logic from this PR alone.
• src/extension/restore.ts - Imported by bin/multicorn-shield.ts but missing from the diff. The restore command is user-facing and critical; without the source, test coverage cannot be assessed.
• package.json:44 - 'build:extension': 'tsup' runs tsup with no arguments — it will use whatever tsup.config is present. Without seeing that config or the source files, it is unclear whether shield-extension.ts is actually included in the build output that gets packaged.
• package.json:45 - The validate:extension and pack:extension scripts depend on 'mcpb' being available globally or as a bin, but @anthropic-ai/mcpb is listed only as a devDependency. CI will work, but a contributor running 'pnpm run validate:extension' without installing devDeps first will get a confusing 'mcpb: command not found' error. A note in CONTRIBUTING or the README would help.
• docs/desktop-extension-research.md - This is internal spike documentation committed to the public repo. It reads like a pre-implementation note (future tense, TODO items) and may confuse contributors about what is shipped vs. planned. Consider moving to an internal wiki or clearly marking it as historical.

Sarah (Non-Technical Decision-Maker)

• README.md:71 - 'TODO: replace with a proper PNG export from the Multicorn learn site favicon' — shipping a TODO comment in public-facing documentation signals the product is unfinished and erodes confidence for a non-technical buyer.
• README.md:57 - The phrase 'merges their tools, and checks every tools/call with the Shield API' uses raw MCP protocol terminology (tools/call) that will be opaque to a non-developer evaluating whether Shield is trustworthy.
• CHANGELOG.md:10 - '[0.2.2] - Unreleased' appears immediately after '[Unreleased]' as a separate section, which is confusing. These should be consolidated or the versioning intent clarified so users know what is actually available.

The Team (Acquisition Due Diligence)

• src/extension/server.ts - Core extension server is not in the diff. For due diligence, the child-process spawning strategy, MCP protocol multiplexing logic, and error handling are the highest-risk components and cannot be evaluated.
• package.json:65 - zod and @modelcontextprotocol/sdk are moved to production dependencies. The SDK pulls in a substantial transitive closure (hono, inquirer, ajv, accepts, etc. visible in pnpm-lock.yaml). For a security/permissions product, each added production dependency is an expanded attack surface. A formal dependency review is warranted.
• pnpm-lock.yaml - @hono/node-server appears as a transitive dependency of @modelcontextprotocol/sdk. Hono is a web framework — its presence in an MCP stdio proxy is unexpected and should be confirmed as genuinely required rather than an over-broad peer dependency being pulled in unnecessarily.
• manifest.json:10 - manifest.json version is 0.2.1 but CHANGELOG targets 0.2.2. Version skew between manifest and package at ship time is a hygiene red flag for acquirers assessing release discipline.
• .github/workflows/ci.yml - The new CI step 'Validate MCPB extension manifest' runs mcpb validate but there is no step that runs tests for the new extension source files. If src/extension/* has no test coverage, this is a gap in the test coverage signal.
• docs/desktop-extension-research.md - Committing spike/research documents to the main branch of a production repo is an OSS health anti-pattern. It suggests an ad-hoc process and will confuse future contributors about the canonical design record.

Yuki (International User)

• README.md:57 - 'checks every tools/call with the Shield API' — 'tools/call' is a raw MCP protocol method name used as plain English. Non-native readers will not know if this is a typo, a technical term, or a file path. Rephrase to 'checks every tool call request'.
• README.md:71 - 'TODO: replace with a proper PNG export' is visible in the public README. This is confusing for any reader — it looks like an instruction directed at developers that was accidentally left in. Remove before publishing.
• bin/multicorn-shield.ts:22 - The help text says 'Restore MCP servers in claude_desktop_config.json from the Shield extension backup' but does not say where the backup file is or what happens if it does not exist. An actionable message would say: 'Reads ~/.multicorn/extension-backup.json and writes the saved MCP server list back into Claude Desktop config. Exits with an error if no backup is found.'
• CHANGELOG.md:13 - 'Packages Shield as a Desktop Extension that wraps existing MCP servers' — 'wraps' is informal. Prefer 'connects to' or 'manages' for clarity in non-native English contexts.

Open-Source Readiness Checklist

Code Quality

• All functions have clear, descriptive names — Functions like runShieldExtension, restoreClaudeDesktopMcpFromBackup, and main are clearly named and descriptive.
• No hardcoded secrets, API keys, internal URLs, or employee names in code or comments — API key is referenced via ${user_config.api_key} substitution, not hardcoded. Public URLs only (multicorn.ai, github.com).
• No // TODO without a public issue reference — README.md contains 'TODO: replace with a proper PNG export from the Multicorn learn site favicon' with no public issue reference linked.
• No commented-out code blocks — No commented-out code blocks observed in the diff.
• No debug logging (console.log, println) left in — All output uses process.stderr.write, which is intentional CLI/server output, not debug logging.
• All any types eliminated (TypeScript) — Error parameters are typed as unknown and narrowed; no untyped any observed in the diff.
• Error handling is complete — no swallowed exceptions, no empty catch blocks — Both entry points catch errors and exit with code 1 and a message. No empty catch blocks visible.
• No Atlassian-internal references, no proprietary patterns or terminology — No Atlassian references found. References are to Anthropic (mcpb tooling), which is appropriate given the integration context.

Testing

• All new code has tests — New source files bin/multicorn-shield.ts, bin/shield-extension.ts, and the referenced src/extension/server.ts and src/extension/restore.ts are added but no corresponding test files appear in the diff.
• [~] Coverage meets or exceeds repo minimum — Coverage report not visible in the diff; CI results not available here.
• [~] Tests pass locally and in CI — CI outcome cannot be determined from the diff alone.
• Edge cases and error paths are tested — No tests visible for edge cases such as missing backup file, malformed claude_desktop_config.json, or duplicate tool names.
• [~] No flaky tests — Cannot assess flakiness from the diff alone.

Security

• No secrets in code, comments, config files, or git history — API key uses ${user_config.api_key} placeholder; no literal secrets present.
• [~] All user input is validated — The src/extension/restore.ts and server.ts implementations are not fully visible in the truncated diff; cannot assess input validation completeness.
• [~] Dependencies audited — no known vulnerabilities — New dependencies (@modelcontextprotocol/sdk, zod, @anthropic-ai/mcpb) added but no audit output is visible.
• HTTPS enforced for all external communication — All URLs in manifest.json and README use HTTPS (multicorn.ai, github.com, app.multicorn.ai).
• [~] API keys/tokens never logged — The server.ts implementation is not visible in the truncated diff; cannot fully verify.

Documentation

• README.md is accurate and up to date — README updated with a Claude Desktop Extension section covering install, disable/restore, duplicate tool names, and local build instructions.
• [~] CONTRIBUTING.md is accurate and up to date — CONTRIBUTING.md not shown in the diff.
• CHANGELOG.md updated with this change — CHANGELOG.md updated with a [0.2.2] Unreleased entry describing the new .mcpb extension and restore command.
• New public APIs have JSDoc/KDoc with examples — Both new bin entry points have JSDoc @module comments. Full API docs for server.ts and restore.ts are not visible but entry points are documented.
• Any new config options are documented — The user_config api_key field is documented in manifest.json and the README explains how it is used.
• Architecture decisions documented in ADR if significant — docs/desktop-extension-research.md serves as a spike/ADR documenting the key architectural decisions and tradeoffs for the .mcpb proxy approach.

Open Source Hygiene

• [~] Licence header present in source files (if required by licence) — MIT licence is declared in manifest.json and package.json, but whether per-file headers are required cannot be determined from the diff alone.
• [~] CODE_OF_CONDUCT.md present — Not touched or created in this diff; cannot confirm presence or absence.
• [~] Issue templates are current — No .github/ISSUE_TEMPLATE changes in this diff.
• [~] PR template is current — No .github/PULL_REQUEST_TEMPLATE changes in this diff.
• No internal company references or links — All references are to public Multicorn/Anthropic properties. SR-04 referenced in docs/desktop-extension-research.md looks like an internal ticket reference.
• Package name and description are correct in package.json — New bin entry multicorn-shield and updated scripts appear consistent with the package purpose.
• [~] Repository topics/tags are set on GitHub — Cannot verify GitHub repository topics from the diff.

---

Advisory only. Does not block merge. Actions logged to Shield as pr_review and oss_check.