Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion workspace-server/internal/handlers/a2a_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"database/sql"
"encoding/json"
"errors"
"fmt"
"io"
"log"
"net/http"
Expand Down
55 changes: 25 additions & 30 deletions workspace-server/internal/handlers/terminal.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,40 +55,17 @@ func NewTerminalHandler(cli *client.Client) *TerminalHandler {
return &TerminalHandler{docker: cli}
}

// HandleConnect handles WS /workspaces/:id/terminal. Routes to the remote
// path (aws ec2-instance-connect ssh + docker exec) when the workspace row
// has an instance_id; falls back to local Docker otherwise.
func (h *TerminalHandler) HandleConnect(c *gin.Context) {
workspaceID := c.Param("id")
ctx := c.Request.Context()

// Check for CP-provisioned workspace (instance_id persisted by
// provisionWorkspaceCP → migration 038). Null instance_id means the
// workspace runs as a local Docker container on this tenant.
var instanceID string
db.DB.QueryRowContext(ctx,
`SELECT COALESCE(instance_id, '') FROM workspaces WHERE id = $1`,
workspaceID).Scan(&instanceID)

if instanceID != "" {
h.handleRemoteConnect(c, workspaceID, instanceID)
return
}

h.handleLocalConnect(c, workspaceID)
}

// handleLocalConnect attaches to a Docker container running on this
// tenant's Docker daemon. Original behavior preserved exactly.
func (h *TerminalHandler) handleLocalConnect(c *gin.Context, workspaceID string) {
// canCommunicateCheck is the communication-authorization predicate used by
// HandleConnect to enforce the KI-005 workspace-hierarchy guard.
// Exposed as a package var so tests can stub it without DB fixtures.
var canCommunicateCheck = registry.CanCommunicate

// HandleConnect handles WS /workspaces/:id/terminal
// HandleConnect handles WS /workspaces/:id/terminal. Routes to the remote
// path (aws ec2-instance-connect ssh + docker exec) when the workspace row
// has an instance_id; falls back to local Docker otherwise. Both paths are
// guarded by the KI-005 CanCommunicate check before dispatch.
func (h *TerminalHandler) HandleConnect(c *gin.Context) {
targetID := c.Param("id")
workspaceID := c.Param("id")
ctx := c.Request.Context()

// KI-005 fix: enforce CanCommunicate hierarchy check before granting
Expand All @@ -103,21 +80,39 @@ func (h *TerminalHandler) HandleConnect(c *gin.Context) {
tok := wsauth.BearerTokenFromHeader(c.GetHeader("Authorization"))
if tok != "" {
if err := wsauth.ValidateAnyToken(ctx, db.DB, tok); err == nil {
if !canCommunicateCheck(callerID, targetID) {
if !canCommunicateCheck(callerID, workspaceID) {
c.JSON(http.StatusForbidden, gin.H{"error": "not authorized to access this workspace's terminal"})
return
}
}
}
}

// Check for CP-provisioned workspace (instance_id persisted by
// provisionWorkspaceCP → migration 038). Null instance_id means the
// workspace runs as a local Docker container on this tenant.
var instanceID string
db.DB.QueryRowContext(ctx,
`SELECT COALESCE(instance_id, '') FROM workspaces WHERE id = $1`,
workspaceID).Scan(&instanceID)

if instanceID != "" {
h.handleRemoteConnect(c, workspaceID, instanceID)
return
}

h.handleLocalConnect(c, workspaceID)
}

// handleLocalConnect attaches to a Docker container running on this
// tenant's Docker daemon. Original behavior preserved exactly.
func (h *TerminalHandler) handleLocalConnect(c *gin.Context, workspaceID string) {
if h.docker == nil {
c.JSON(http.StatusServiceUnavailable, gin.H{"error": "Docker not available"})
return
}

ctx := c.Request.Context()
workspaceID := targetID

// Try multiple container name patterns:
// 1. Provisioner naming: ws-{id[:12]}
Expand Down
6 changes: 6 additions & 0 deletions workspace-server/internal/handlers/terminal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ func TestHandleConnect_RoutesToLocal(t *testing.T) {

if w.Code != http.StatusServiceUnavailable {
t.Errorf("local branch should 503 when Docker is unavailable; got %d", w.Code)
}
}

// TestTerminalConnect_KI005_RejectsUnauthorizedCrossWorkspace tests the KI-005
// regression fix: workspace A must NOT be able to open a terminal on workspace B's
// container, even with a valid bearer token, unless they share a parent/child
Expand Down Expand Up @@ -144,6 +147,9 @@ func TestSSHCommandCmd_BuildsArgv(t *testing.T) {
if cmd.Args[i] != want[i] {
t.Errorf("argv[%d] = %q, want %q", i, cmd.Args[i], want[i])
}
}
}

// TestTerminalConnect_KI005_AllowsOwnTerminal tests the flip side of KI-005:
// a workspace must still be able to access its own terminal. The CanCommunicate
// fast-path returns true when callerID == targetID.
Expand Down
19 changes: 0 additions & 19 deletions workspace-server/internal/handlers/workspace_crud.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,15 @@ package handlers
// Delete (cascade + purge), and input validation helpers.

import (
"context"
"database/sql"
"fmt"
"log"
"net/http"
"path/filepath"
"strings"

"github.com/Molecule-AI/molecule-monorepo/platform/internal/crypto"
"github.com/Molecule-AI/molecule-monorepo/platform/internal/db"
"github.com/Molecule-AI/molecule-monorepo/platform/internal/wsauth"
"github.com/Molecule-AI/molecule-monorepo/platform/pkg/provisionhook"
"github.com/gin-gonic/gin"
"github.com/google/uuid"
"github.com/lib/pq"
Expand Down Expand Up @@ -137,22 +134,6 @@ func (h *WorkspaceHandler) Update(c *gin.Context) {
return
}

// #685/#688: validate string fields for length and injection safety.
strField := func(key string) string {
if v, ok := body[key]; ok {
if s, ok := v.(string); ok {
return s
}
}
return ""
}
if err := validateWorkspaceFields(
strField("name"), strField("role"), "" /*model not patchable*/, strField("runtime"),
); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid workspace fields"})
return
}

// #685/#688: validate string fields for length and injection safety.
strField := func(key string) string {
if v, ok := body[key]; ok {
Expand Down
Loading