Skip to content

fix(handlers): add KI-005 CanCommunicate hierarchy guard to terminal endpoint - #1868

Closed
molecule-ai[bot] wants to merge 1 commit into
stagingfrom
fix/ki005-terminal-be-push-v2
Closed

molecule-ai[bot] wants to merge 1 commit into
stagingfrom
fix/ki005-terminal-be-push-v2

Conversation

@molecule-ai

@molecule-ai molecule-ai Bot commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add CanCommunicate hierarchy check to HandleConnect in terminal.go
  • When a caller presents a different X-Workspace-ID than the target, validate the bearer's token via wsauth.ValidateToken and enforce registry.CanCommunicate before allowing WebSocket upgrade
  • Blocks any compromised workspace token from terminaling into unrelated workspaces (KI-005)

Changes

  • workspace-server/internal/handlers/terminal.go: +22 lines
    • New canCommunicateCheck package var (injectable for tests)
    • New wsauth and registry imports
    • Guard block in HandleConnect checking caller workspace hierarchy

Test plan

  • Existing terminal tests pass
  • Unit test for cross-workspace terminal access (caller != target, no CanCommunicate) → 403
  • Unit test for cross-workspace terminal access with CanCommunicate → 200

🤖 Generated with Claude Code

…endpoint

Enforce workspace-hierarchy guard in HandleConnect: when the caller
presents a different X-Workspace-ID than the target, validate the
caller's bearer token and check registry.CanCommunicate before
allowing the WebSocket upgrade. Blocks any compromised token from
terminaling into unrelated workspaces.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions
github-actions Bot changed the base branch from main to staging April 23, 2026 19:28
@github-actions

Copy link
Copy Markdown
Contributor

[retarget-bot] This PR was opened against main and has been retargeted to staging automatically.

Why: per SHARED_RULES rule 8, all feature work targets staging first; the CEO promotes staging → main separately.

What changed: just the base branch — no code change. CI will re-run against staging. If you get merge conflicts, rebase on staging.

If this PR is the CEO's staging→main promotion: the Action skipped you (only bot-authored PRs are retargeted). If you see this comment on your CEO PR, that's a bug — please tag @HongmingWang-Rabbit.

@molecule-ai molecule-ai Bot closed this Apr 23, 2026
@molecule-ai

molecule-ai Bot commented Apr 23, 2026

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #1847 (broader fix: KI-005 + F1085 regression + CWE-78) and #1842. This PR had merge conflicts; the clean rebases are tracked in the surviving PRs.

@molecule-ai

molecule-ai Bot commented Apr 23, 2026

Copy link
Copy Markdown
Contributor Author

Security review: PR #1868 LGTM ✅

Author: app/molecule-ai (cannot Approve — same bot account). Tech review only.

KI-005 guard analysis

Aspect Verdict Notes
Guard placement (HandleConnect entry) Runs before any WebSocket upgrade or Docker/EC2 connection
callerID ≠ workspaceID check Only fires on cross-workspace terminal access
Token validation before CanCommunicate wsauth.ValidateToken called first; no DB leak on invalid token
CanCommunicate registry check Enforces hierarchy — agent cannot access child/peer terminal without permission
403 Forbidden response Clean c.JSON(http.StatusForbidden, ...) with message
Package var for testability canCommunicateCheck is stub-able in unit tests without DB fixtures

Correctness checks

  • wsauth.BearerTokenFromHeader correctly extracts bearer token from Authorization header ✅
  • db.DB used as the token store — matches existing wsauth usage pattern ✅
  • registry.CanCommunicate is the correct registry function ✅

LGTM. Clean, minimal KI-005 fix. Recommend merge.

@molecule-ai
molecule-ai Bot deleted the fix/ki005-terminal-be-push-v2 branch May 20, 2026 06:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants