feat: per-identity credential management with groups and RBAC

[rodaddy]

Summary

• Adds per-identity credential mapping so different users/agents get their own API keys for backend MCP services
• Resolution chain: user-specific → group → defaults → services.json fallback
• Full CLI, daemon API, RBAC permissions, and security hardening included
• 5-reviewer swarm review completed with all CRITICAL/HIGH/MEDIUM findings fixed

What's New

Core:

• credentials.json schema with groups, per-identity credentials, and defaults
• CredentialManager class with async write mutex, Zod validation, disk persistence (0600 perms)
• Per-user connection pooling (service::userId keys) with baseServiceNames for health/metrics
• Credential merge into service configs at /call, /list-tools, /schema time
• Pool invalidation on credential changes via closeServicePattern()

API (12 routes, extracted to src/daemon/routes/credentials.ts):

• GET/POST/DELETE /api/credentials — list (redacted), set, remove
• GET /api/credentials/resolve — resolve effective credential (IDOR-protected)
• POST/DELETE /api/credentials/defaults — set/remove defaults
• GET/POST /api/credentials/groups — list/create groups
• PUT/DELETE /api/credentials/groups/:name — add/remove members or delete group
• POST /api/credentials/reload — reload from disk

CLI:

• mcp2cli credentials list|set|set-default|remove|remove-default|resolve|group|reload

RBAC:

• credentials-read (agent+): list, resolve own credentials
• credentials-write (admin only): set, remove, group management

Security (from swarm review):

• Redacted GET responses (first 4 chars + ***)
• IDOR protection on resolve (agents can only resolve own userId)
• Header injection prevention (CRLF rejection, dangerous header blocklist)
• Env var injection prevention (PATH, LD_PRELOAD, NODE_OPTIONS blocklist)
• Malformed JSON handling (SyntaxError → CredentialManagerError)
• File permissions (0600 on credentials.json)

Test plan

• 61 new tests across 4 test files (schema, credential-manager, credential-merge, rbac)
• Full suite: 930 tests pass, 0 failures
• Clean typecheck (bunx tsc --noEmit)
• 5-reviewer swarm review: correctness, adversarial, quality, security, backend specialist
• Manual: mcp2cli credentials set rico open-brain --header "Authorization: Bearer xxx"
• Manual: mcp2cli credentials resolve rico open-brain
• Manual: verify credential injection on actual tool call

🤖 Generated with Claude Code

[github-actions]

Claude Code is working…

I'll analyze this and get back to you.

View job run

[rodaddy]

🔍 Review Swarm Report — Round 1

5 reviewers: Correctness, Adversarial Critic, Quality Analyst, Security Auditor, Backend Specialist

CRITICAL (1)

ID	File	Description	Flagged By
C-1	pool.ts:109-115	Circuit breaker uses poolKey (service::userId) instead of base service name — fragments fault detection per-user, delays fallback activation. N users = N independent failure detections instead of 1.	Adversarial, Quality, Backend

HIGH (4)

ID	File	Description	Flagged By
H-1	credential-manager.ts:319	Write queue poison pill — one disk write failure permanently breaks all subsequent writes (rejected promise chain never recovers)	Correctness, Quality, Backend
H-2	routes/credentials.ts:43	/api/credentials/resolve returns unredacted secrets to any agent-role user. Compromised agent token = full credential exfiltration.	All 5 reviewers
H-3	routes/credentials.ts:48-55	POST/PUT routes miss JSON parse error handling — malformed JSON returns 500 INTERNAL_ERROR instead of 400. DELETE routes do it correctly.	Correctness, Quality, Backend, Adversarial
H-4	credential-manager.ts:237	removeGroup() leaves orphaned credentials. New group with same name silently inherits stale creds — potential privilege escalation.	Quality, Adversarial

MEDIUM (9)

ID	File	Description	Flagged By
M-1	credential-manager.ts:143-150	In-memory state mutated before disk write — diverges on failure, lost on restart	Adversarial, Backend
M-2	server.ts:126-140	Credential resolution logic copy-pasted 3x in /call, /list-tools, /schema — divergence risk	Quality, Backend, Adversarial
M-3	schema.ts:10-17	Env var blocklist incomplete — missing DYLD_INSERT_LIBRARIES, MCP2CLI_* self-referential vars, NODE_PATH, PYTHONPATH, HOME	Security, Adversarial
M-4	credential-manager.ts:310-315	TOCTOU race — Bun.write() then chmod() leaves brief window where creds are world-readable	Security
M-5	schema.ts	No size limits on credential records — no maxProperties on headers/env, no max length on values	Backend
M-6	credential-manager.ts:78-86	Group resolution order depends on JSON key insertion order — undocumented, fragile	Quality, Adversarial
M-7	schema.test.ts	Zero test coverage for injection prevention (dangerous headers, env vars, CRLF)	Security, Adversarial
M-8	rbac.test.ts	Tests hasPermission() but not path-to-permission mapping (checkPermission())	Adversarial
M-9	pool.ts	Per-user pool keys can exhaust pool limit (10 services × 5 users = 50 = default max)	Backend

LOW / INFO (not blocking)

• Redaction leaks 50% of short secrets (first 4 chars of 8-char key)
• No rate limiting on /api/auth/login
• fetchDaemonApi doesn't check response.ok before parsing JSON
• No version bump or CHANGELOG entry
• Existing pool tests not updated for new getConnection signature
• Credential env merge silently overrides service-level vars (no audit log)
• Default-allow on unmatched auth paths
• Group name path parameter has no length/character validation

Validation

• TypeScript: bunx tsc --noEmit ✅ PASS
• Tests: 61/61 credential tests pass ✅
• Secrets scan (ggshield): PASS ✅
• CI: PASS ✅

Security Checklist

Check	Status
Secrets in code	✅ PASS
Auth on new endpoints	✅ 12 routes have RBAC rules
IDOR protection	✅ Agents can only resolve own userId
Input validation (Zod)	✅ All writes validated
CRLF injection	✅ Header values reject \r \n
Header injection	✅ Dangerous headers blocked (case-insensitive)
Env injection	⚠️ PARTIAL — core vars blocked, list incomplete
File permissions	⚠️ PARTIAL — 0600 applied but TOCTOU window
Timing-safe auth	✅ Both token and basic auth
Credential redaction on GET	✅ List endpoint redacts
Error message leakage	✅ No stack traces or internals
Audit logging	✅ All mutations logged, no values
Rate limiting	❌ MISSING

---

Fixer agent dispatched for all CRITICAL/HIGH/MEDIUM findings. Re-review pending.

🤖 Generated by review-swarm (5 parallel CC agents)

[github-actions]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.

[github-actions]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.

[rodaddy]

✅ Review Swarm — Round 2: All Findings Fixed

Re-reviewer verified all 14 CRITICAL/HIGH/MEDIUM findings from Round 1 are resolved. 946 tests pass, typecheck clean, no regressions.

Fixes Applied

ID	Severity	Fix Summary
C-1	CRITICAL	Circuit breaker now uses serviceName.split("::")[0] for shouldAttemptHttp/recordSuccess/recordFailure — per-service fault detection instead of per-user fragmentation
H-1	HIGH	writeToDisk() separates queue chain (op.catch(() => {})) from caller promise (return op) — queue stays alive on failure, caller still gets error
H-2	HIGH	/api/credentials/resolve now returns { exists: true/false, source: "user"|"group"|"default"|null } — no raw credential values exposed
H-3	HIGH	All 7 POST/PUT routes now wrap req.json() in nested try/catch returning 400 "Malformed JSON body" — matches existing DELETE pattern
H-4	HIGH	removeGroup() now also deletes credentials[groupName] — prevents orphaned credentials from being inherited by future groups
M-1	MEDIUM	All 8 write methods snapshot this.config via structuredClone() before mutation, restore on disk write failure
M-2	MEDIUM	Extracted resolveCredentialPool() helper in server.ts — replaces 3 identical 8-line blocks in /call, /list-tools, /schema
M-3	MEDIUM	DANGEROUS_ENV_VARS expanded: +DYLD_INSERT_LIBRARIES, NODE_PATH, PYTHONPATH, HOME, MCP2CLI_AUTH_TOKEN, MCP2CLI_CREDENTIALS_FILE, MCP2CLI_TOKENS_FILE
M-4	MEDIUM	Atomic write: Bun.write(tmpPath) → chmod(0o600) → rename(tmpPath, configPath) — eliminates TOCTOU window
M-5	MEDIUM	Size limits: max 50 entries per headers/env record, 256-char key limit, 8192-char value limit
M-6	MEDIUM	resolve() JSDoc now documents group iteration follows insertion order, earlier groups take precedence
M-7	MEDIUM	+6 tests: dangerous header names, CRLF injection, dangerous env vars (including new blocklist entries), max-entry-count limits
M-8	MEDIUM	+68 lines: checkPermission() path-mapping tests for all 11 credential routes × 3 roles (admin, agent, viewer)
M-9	MEDIUM	Pool logs warning at 80% capacity mentioning per-user credential connections; error message suggests MCP2CLI_POOL_MAX

Files Changed (fixer + cleanup)

 src/credentials/credential-manager.ts  | 118 ++++++++++-----
 src/credentials/schema.ts              |  23 ++-
 src/daemon/pool.ts                     |  36 +++--
 src/daemon/routes/credentials.ts       |  35 ++++-
 src/daemon/server.ts                   |  57 +++----
 tests/credentials/rbac.test.ts         |  68 +++++++++
 tests/credentials/schema.test.ts       |  32 ++++
 7 files changed, 281 insertions(+), 88 deletions(-)

Validation

Check	Result
bunx tsc --noEmit	✅ Clean
bun test	✅ 946 pass, 0 fail, 2062 assertions
Regressions	✅ None detected
File sizes	✅ All under 750 lines

🤖 Generated by review-swarm (Round 2 re-review)

[rodaddy]

Review swarm follow-up

I ran a 3-round review swarm on PR #28 with five lanes each round:

• correctness
• adversarial
• quality/maintainability
• security
• backend/API domain

Pinned diffs used during the swarm:

• Round 1: /tmp/mcp2cli-pr28-review-swarm-round1.diff, SHA-256 fde89ca291cba75b6be0bf7e53bcdba42dd6d04a684b7f8f00f8126336f4d448
• Round 2: /tmp/mcp2cli-pr28-review-swarm-round2.diff, SHA-256 67a0d70bff3c5cd820a0f675a699e21c3f6feab9d107d5dd8b942d5ed60fbbdc
• Round 3: /tmp/mcp2cli-pr28-review-swarm-round3.diff, SHA-256 9827aecd90b928f2209a78028ce0c5fee4942a99c8fcc110a48070e8643f1314

Findings fixed

• Fixed credential merge so HTTP/WebSocket headers and stdio env overrides are created even when the base service config omitted those objects.
• Stopped /api/credentials/resolve from exposing raw resolved credentials; it now returns only existence/source metadata.
• Restricted full credential inventory and group inventory reads to admin-level credential-write permission.
• Fixed stale connection handling for default credential changes by closing the base service connection as well as credential-scoped connections.
• Added eviction for group membership changes, group removal, credential reload, and credential updates.
• Added credential-scoped schema cache invalidation so users do not keep stale tool/schema views after credential changes.
• Serialized credential read-modify-write operations through the existing queue so failed writes cannot roll back over concurrent updates.
• Moved reload through the same queue and closes existing pool connections after successful reload.
• Added first-write directory creation and atomic 0600 temp-file persistence for credentials.
• Added credential key validation for reserved object keys, control chars, path separators, query/fragment separators, dangerous headers, and dangerous env vars.
• Preserved intended group credential support while preventing creating a new group over an existing non-group credential identity.
• Replaced ambiguous service::user credential pool/cache keys with encoded tuple keys.
• Changed pool entries to store baseServiceName and made service eviction use that field instead of string-prefix parsing.
• Avoided per-user pool exhaustion for shared default credentials by reusing the base service pool key; group credentials now use a shared group-scoped key.

Tests and verification

• bun run typecheck
• bun test -> 963 pass, 0 fail
• git diff --check
• ggshield secret scan repo . -> no secrets found

Pushed fix commit: f8d0888 (fix: harden credential management after swarm review)