fix(deploy-local): require usable CLAUDE.md + advisory MCP credential-gap warnings (#950)

[AndriiPasternak31]

Summary

Deferred hardening for #950 (the deploy-local empty-agent bug; core fix landed in #971). Two behavior changes plus docs and a CSO audit:

is_trinity_compatible() — CLAUDE.md is now blocking. A deploy archive with no usable CLAUDE.md (missing / empty / whitespace-only / non-UTF-8) returns a clean 400 NOT_TRINITY_COMPATIBLE instead of warning and deploying an instruction-less agent. A binary/non-UTF-8 CLAUDE.md is caught explicitly (UnicodeDecodeError/OSError) so it yields a 400, not an unhandled 500.

collect_mcp_credential_warnings() — advisory credential-gap warnings. Scans .mcp.json.template/.mcp.json for ${VAR} references that are absent from the post-merge .env and not platform-injected (static allowlist mirroring crud.py), surfaced as non-fatal DeployLocalResponse.warnings[] (also added to the MCP deploy_local_agent response type). MCP server names (operator-supplied JSON keys) are sanitized — control-char strip + length bound — before echoing (#950 L1).

docs/ reconciles already-merged #971 behavior too: credentials request field, MAX_DEPLOY_CREDENTIALS, require_role("creator"), and the DEPLOYED_TEMPLATES_DIR_UNWRITABLE/WORKSPACE_PREPOP_FAILED codes.

Closes #950.

Test Coverage

All new code paths covered. Tests: +3 files (2 unit, 1 integration class extension).

• Unit (14, no server): test_deploy_local_validation.py (CLAUDE.md missing/empty/whitespace/binary/valid/unicode; warnings: no-config/satisfied/platform-injected/unsatisfied/multiple/malicious-name/overlong) + test_deploy_writable_templates.py (DEPLOYED_TEMPLATES_DIR_UNWRITABLE fail-fast).
• Integration (3): missing-CLAUDE.md → 400, MCP credential-gap warnings (HEYGEN warned, BLOTATO suppressed via REST merge), valid deploy + workspace prepop.

Verification: tests/test_deploy_local.py needs a backend running this branch. The shared local stack runs a different checkout, so I stood up a sibling backend on this branch (isolated DB, sibling Redis, real Docker socket) and ran the full suite:

16 passed in 91.86s   # full tests/test_deploy_local.py against this branch
14 passed in 0.13s    # tests/unit/test_deploy_local_validation.py + test_deploy_writable_templates.py
tests/lint_sys_modules.py: OK (no new violations)
ruff/mypy: zero new findings in changed code (pre-existing baseline debt untouched)

Pre-Landing Review

No issues. Three-layer boundaries respected (router → service → db); new logic is pure functions in template_service.py. No new dependencies. No auth-surface change (the require_role("creator") gate is pre-existing/documented). Pydantic warnings: List[str] = [] default is safe (Pydantic deep-copies field defaults).

Security (CSO diff audit)

CLEAR — docs/security-reports/cso-diff-2026-05-29-950.md. No secrets, no dependency CVEs, no injection/auth regressions; net security-positive (500→400 crash-path hardening, no-silent-fallback). One LOW finding (L1, operator-supplied MCP server-name terminal-escape echo) resolved in this branch via _sanitize_for_warning() with TDD. Credential values are never read into warnings (variable names only).

Test plan

• 14 unit tests pass (standalone, no server)
• 16/16 test_deploy_local.py pass against a backend running this branch
• sys_modules lint clean; ruff/mypy clean for changed code

🤖 Generated with Claude Code

[AndriiPasternak31]

CI fix: CodeQL py/path-injection (commit e97a4d36)

The only failing check was the PR-level CodeQL gate — 2 new high-severity py/path-injection alerts on the new collect_mcp_credential_warnings() .exists() calls (template_service.py:766/768). All other checks (pytest ×6, lint, build-without-submodule, regression-diff, CodeQL Analyze sub-jobs) were already green.

Why it fired (false positive)

The taint flow is:

body.name → sanitize_agent_name() → version_name → dest_path
          → collect_mcp_credential_warnings(dest_path) → (dir / ".mcp.json.template").exists()

sanitize_agent_name() (utils/helpers.py) replaces every char outside [a-zA-Z0-9_.-] with - and strips leading non-alphanumerics, so the result is a single safe path component that cannot traverse. CodeQL doesn't model that regex as a barrier, so it flags the downstream .exists(). The CSO diff audit (docs/security-reports/cso-diff-2026-05-29-950.md) had already reached a CLEAR verdict on this.

What changed

1. Defense-in-depth guard (deploy.py): added an inline os.path.normpath + startswith containment check at the dest_path construction site — exactly the barrier CodeQL's docs recommend — and flow the normalized Path downstream (returns HTTP 400 TEMPLATE_PATH_ESCAPE on escape). This is genuine hardening at the filesystem-write boundary regardless of the scanner.
2. Sanitizer-invariant test battery (tests/unit/test_deploy_local_validation.py): asserts sanitize_agent_name never yields a separator, a .. component, or a non-basename across 12 hostile inputs (../../etc/passwd, a/../../b, unicode + traversal, RTL override, etc.). Server-free; proves the property that makes the alert a false positive and guards against future regressions to the sanitizer. 15/15 unit tests pass; ruff/mypy/sys.modules-lint clean for the change.

Why the alerts were also dismissed

I verified against the analyzed merge commit (afe57e95, which contains the guard) that CodeQL still flagged both alerts. CodeQL's SafeAccessCheck barrier is intraprocedural — it doesn't propagate across the deploy.py → collect_mcp_credential_warnings call boundary (the guarded value is re-wrapped and the .exists() sink lives in a different function). So the in-code guard alone couldn't turn the gate green. Alerts #179/#180 were dismissed as false positive (reversible, per-alert), backed by three independent confirmations: the CSO audit, the sanitize_agent_name regex, and the new invariant test.

Result: all checks on this PR now pass (CodeQL: ✅).

[github-actions]

⚠️ Nightly unit-suite check skipped — merge conflict against dev.

Resolve by running git merge dev locally and pushing the result. The next nightly run will re-test once the conflict is gone.

[vybe]

Approved via /validate-pr with in-code security verification.

Confirmed in the diff (not just from the description):

• Path-containment guard uses normpath + base + os.sep prefix check — no /foo-evil//foo bypass; normalized value flows downstream.
• is_trinity_compatible decodes CLAUDE.md strictly and catches UnicodeDecodeError/OSError → clean 400, never a 500 fall-through. Single caller (deploy.py:382) so the new blocking behavior does NOT touch the GitHub-template clone path.
• _sanitize_for_warning strips non-printables via isprintable() (ANSI/C0/C1/RTL-override) and bounds length — L1 resolved.
• Credential warnings echo ${VAR} names + sanitized server name only; values are never read in.
• Three-layer boundaries respected; MCP agents.ts response type updated (Invariant #13); feature-flows + CSO audit included; CI 13/13 + 30 tests green.

Non-blocking: the static _PLATFORM_INJECTED_* mirror of crud.py can drift (advisory-only impact) — a parity test would catch it. models.py overlaps #986; sequence the merges so the second rebases.