fix: fall back to name lookup for UUID-shaped workspace names (#24340)

`namedWorkspace` in `cli/root.go` parsed workspace identifiers with
`uuid.Parse` first and returned immediately on success, even when no
workspace had that UUID as its actual ID. This caused 404 errors for any
workspace whose name was a valid 32-char hex string (dashless UUID).

- Add `codersdk.ResolveWorkspace`: tries UUID lookup first, falls back
to name lookup on 404. `NameValid` guard skips the fallback for standard
dashed UUIDs (36 chars > 32-char name limit).
- Export `codersdk.SplitWorkspaceIdentifier`, replacing the duplicate
`splitNamedWorkspace` in `cli/root.go` (uses `strings.Cut`).
- Delete `namedWorkspace` from `cli/root.go`; all 28 call sites now use
`client.ResolveWorkspace` directly.
- Delete `namedWorkspace` and `splitNameAndOwner` from
`codersdk/toolsdk/bash.go`; inline `client.ResolveWorkspace`.
- Simplify `GetWorkspace` tool handler to a single `ResolveWorkspace`
call.
- Unit tests via httptest mock cover UUID, name, owner/name, UUID-like
fallback, not-found, server error, transport error, and invalid
identifier paths.
- Integration tests in `cli/show_test.go` and `codersdk/toolsdk` for
workspaces with UUID-like names.

> Generated with Coder Agents
This commit is contained in:
Cian Johnston
2026-04-27 12:58:26 +01:00
committed by GitHub
parent 23b30b7285
commit d5a5be116d
25 changed files with 498 additions and 102 deletions
+1 -32
View File
@@ -190,7 +190,7 @@ func findWorkspaceAndAgent(ctx context.Context, client *codersdk.Client, workspa
}
// Get workspace
workspace, err := namedWorkspace(ctx, client, workspaceName)
workspace, err := client.ResolveWorkspace(ctx, workspaceName)
if err != nil {
return codersdk.Workspace{}, codersdk.WorkspaceAgent{}, err
}
@@ -274,37 +274,6 @@ func getWorkspaceAgent(workspace codersdk.Workspace, agentName string) (codersdk
return codersdk.WorkspaceAgent{}, xerrors.Errorf("multiple agents found, please specify the agent name, available agents: %v", availableNames)
}
func splitNameAndOwner(identifier string) (name string, owner string) {
// Parse owner and name (workspace, task).
parts := strings.SplitN(identifier, "/", 2)
if len(parts) == 2 {
owner = parts[0]
name = parts[1]
} else {
owner = "me"
name = identifier
}
return name, owner
}
// namedWorkspace gets a workspace by owner/name or just name
func namedWorkspace(ctx context.Context, client *codersdk.Client, identifier string) (codersdk.Workspace, error) {
workspaceName, owner := splitNameAndOwner(identifier)
// Handle -- separator format (convert to / format)
if strings.Contains(identifier, "--") && !strings.Contains(identifier, "/") {
dashParts := strings.SplitN(identifier, "--", 2)
if len(dashParts) == 2 {
owner = dashParts[0]
workspaceName = dashParts[1]
}
}
return client.WorkspaceByOwnerAndName(ctx, owner, workspaceName, codersdk.WorkspaceOptions{})
}
// executeCommandWithTimeout executes a command with timeout support
func executeCommandWithTimeout(ctx context.Context, session *gossh.Session, command string) ([]byte, error) {
// Set up pipes to capture output
+1 -5
View File
@@ -432,11 +432,7 @@ This returns more data than list_workspaces to reduce token usage.`,
},
MCPAnnotations: mcpReadOnlyAnnotations,
Handler: func(ctx context.Context, deps Deps, args GetWorkspaceArgs) (codersdk.Workspace, error) {
wsID, err := uuid.Parse(args.WorkspaceID)
if err != nil {
return namedWorkspace(ctx, deps.coderClient, NormalizeWorkspaceInput(args.WorkspaceID))
}
return deps.coderClient.Workspace(ctx, wsID)
return deps.coderClient.ResolveWorkspace(ctx, NormalizeWorkspaceInput(args.WorkspaceID))
},
}
+52 -1
View File
@@ -45,6 +45,14 @@ import (
// nolint:gocritic // This is in a test package and does not end up in the build
func setupWorkspaceForAgent(t *testing.T, opts *coderdtest.Options) (*codersdk.Client, database.WorkspaceTable, string) {
t.Helper()
return setupWorkspaceForAgentWithName(t, opts, "myworkspace")
}
// setupWorkspaceForAgentWithName creates a workspace setup exactly like main
// SSH tests, but with a caller-provided workspace name.
// nolint:gocritic // This is in a test package and does not end up in the build
func setupWorkspaceForAgentWithName(t *testing.T, opts *coderdtest.Options, workspaceName string) (*codersdk.Client, database.WorkspaceTable, string) {
t.Helper()
client, store := coderdtest.NewWithDatabase(t, opts)
client.SetLogger(testutil.Logger(t).Named("client"))
@@ -54,7 +62,7 @@ func setupWorkspaceForAgent(t *testing.T, opts *coderdtest.Options) (*codersdk.C
})
// nolint:gocritic // This is in a test package and does not end up in the build
r := dbfake.WorkspaceBuild(t, store, database.WorkspaceTable{
Name: "myworkspace",
Name: workspaceName,
OrganizationID: first.OrganizationID,
OwnerID: user.ID,
}).WithAgent().Do()
@@ -241,6 +249,31 @@ func TestTools(t *testing.T) {
}
})
t.Run("GetWorkspace_ByUUIDLikeName", func(t *testing.T) {
t.Parallel()
// Regression test: a workspace whose name is a valid dashless
// UUID should resolve correctly. Previously, the handler would
// parse the name as a UUID, get a 404 from the ID-based lookup,
// and never fall back to name-based lookup.
const uuidLikeName = "a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6"
// nolint:gocritic // This is in a test package and does not end up in the build
uuidWorkspace := dbfake.WorkspaceBuild(t, store, database.WorkspaceTable{
OrganizationID: owner.OrganizationID,
OwnerID: member.ID,
Name: uuidLikeName,
}).Do()
tb, err := toolsdk.NewDeps(memberClient)
require.NoError(t, err)
result, err := testTool(t, toolsdk.GetWorkspace, tb, toolsdk.GetWorkspaceArgs{
WorkspaceID: uuidLikeName,
})
require.NoError(t, err)
require.Equal(t, uuidWorkspace.Workspace.ID, result.ID)
})
t.Run("ListTemplates", func(t *testing.T) {
tb, err := toolsdk.NewDeps(memberClient)
require.NoError(t, err)
@@ -566,6 +599,24 @@ func TestTools(t *testing.T) {
require.NoError(t, err)
require.Equal(t, 0, result.ExitCode)
require.Equal(t, "owner format works", result.Output)
// Regression test: agent-backed tools should also work when the
// workspace name is a valid dashless UUID.
const uuidLikeName = "a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6"
uuidClient, uuidWorkspace, uuidAgentToken := setupWorkspaceForAgentWithName(t, nil, uuidLikeName)
_ = agenttest.New(t, uuidClient.URL, uuidAgentToken)
coderdtest.NewWorkspaceAgentWaiter(t, uuidClient, uuidWorkspace.ID).Wait()
uuidTB, err := toolsdk.NewDeps(uuidClient)
require.NoError(t, err)
result, err = testTool(t, toolsdk.WorkspaceBash, uuidTB, toolsdk.WorkspaceBashArgs{
Workspace: uuidWorkspace.Name,
Command: "echo 'uuid-like name works'",
})
require.NoError(t, err)
require.Equal(t, 0, result.ExitCode)
require.Equal(t, "uuid-like name works", result.Output)
})
t.Run("WorkspaceLS", func(t *testing.T) {
+48
View File
@@ -3,6 +3,7 @@ package codersdk
import (
"context"
"encoding/json"
"errors"
"fmt"
"net/http"
"net/http/cookiejar"
@@ -612,6 +613,53 @@ func (c *Client) WorkspaceByOwnerAndName(ctx context.Context, owner string, name
return workspace, json.NewDecoder(res.Body).Decode(&workspace)
}
// SplitWorkspaceIdentifier splits an identifier into owner and
// workspace name. A bare name defaults the owner to Me ("me"). An
// "owner/name" pair is accepted, and identifiers with more than one
// "/" are rejected.
func SplitWorkspaceIdentifier(identifier string) (owner, name string, err error) {
owner, name, ok := strings.Cut(identifier, "/")
if !ok {
return Me, identifier, nil
}
if strings.Contains(name, "/") {
return "", "", xerrors.Errorf("invalid workspace identifier: %q", identifier)
}
return owner, name, nil
}
// ResolveWorkspace fetches a workspace by identifier, which may be a
// UUID, a bare name (owned by the current user), or an "owner/name"
// pair. When the identifier parses as a valid UUID but no workspace
// exists with that ID, the function falls back to a name-based
// lookup because workspace names can be valid UUID strings.
func (c *Client) ResolveWorkspace(ctx context.Context, identifier string) (Workspace, error) {
if uid, err := uuid.Parse(identifier); err == nil {
ws, err := c.Workspace(ctx, uid)
if err == nil {
return ws, nil
}
// A workspace name might be a valid UUID string. If the
// ID-based lookup returned 404, fall through to name-based
// lookup below.
var sdkErr *Error
if !errors.As(err, &sdkErr) || sdkErr.StatusCode() != http.StatusNotFound {
return Workspace{}, err
}
// A standard dashed UUID (36 chars) cannot be a valid
// workspace name (max 32 chars). Skip the wasted
// name-based round-trip.
if err := NameValid(identifier); err != nil {
return Workspace{}, sdkErr
}
}
owner, name, err := SplitWorkspaceIdentifier(identifier)
if err != nil {
return Workspace{}, err
}
return c.WorkspaceByOwnerAndName(ctx, owner, name, WorkspaceOptions{})
}
type WorkspaceQuota struct {
CreditsConsumed int `json:"credits_consumed"`
Budget int `json:"budget"`
+310
View File
@@ -0,0 +1,310 @@
package codersdk_test
import (
"encoding/json"
"errors"
"net/http"
"net/http/httptest"
"net/url"
"sync/atomic"
"testing"
"github.com/go-chi/chi/v5"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/coder/coder/v2/codersdk"
)
func TestResolveWorkspace(t *testing.T) {
t.Parallel()
// writeJSON is a small helper that writes a JSON-encoded value
// with the given status code.
writeJSON := func(w http.ResponseWriter, status int, v any) {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(status)
_ = json.NewEncoder(w).Encode(v)
}
// errResponse builds a codersdk.Response suitable for error
// replies.
errResponse := func(msg string) codersdk.Response {
return codersdk.Response{Message: msg}
}
// newWorkspace returns a Workspace with the given ID and name.
newWorkspace := func(id uuid.UUID, name string) codersdk.Workspace {
return codersdk.Workspace{ID: id, Name: name}
}
// Each table case configures a mock server with separate UUID
// and name endpoint behaviors, then calls ResolveWorkspace with
// the given identifier.
type endpointResponse struct {
status int
workspace codersdk.Workspace
errMsg string
}
tests := []struct {
name string
identifier string
// uuidEndpoint configures GET /api/v2/workspaces/{workspace}.
// nil means the endpoint is not registered (404 from chi).
uuidEndpoint *endpointResponse
// nameEndpoint configures GET /api/v2/users/{user}/workspace/{workspace}.
// nil means the endpoint is not registered.
nameEndpoint *endpointResponse
// expectedOwner and expectedName are checked via assert inside
// the name endpoint handler (when non-empty).
expectedOwner string
expectedName string
// Expected outcomes.
wantErr bool
wantStatusCode int
wantUUIDHits int64
wantNameHits int64
}{
{
name: "ByUUID",
identifier: "", // filled dynamically below
uuidEndpoint: &endpointResponse{
status: http.StatusOK,
},
wantUUIDHits: 1,
wantNameHits: 0,
},
{
name: "ByName",
identifier: "my-workspace",
nameEndpoint: &endpointResponse{
status: http.StatusOK,
},
expectedOwner: "me",
expectedName: "my-workspace",
wantUUIDHits: 0,
wantNameHits: 1,
},
{
name: "ByOwnerAndName",
identifier: "alice/my-workspace",
nameEndpoint: &endpointResponse{
status: http.StatusOK,
},
expectedOwner: "alice",
expectedName: "my-workspace",
wantUUIDHits: 0,
wantNameHits: 1,
},
{
name: "OwnerWithUUIDLikeName",
identifier: "alice/a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6",
nameEndpoint: &endpointResponse{
status: http.StatusOK,
},
expectedOwner: "alice",
expectedName: "a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6",
wantUUIDHits: 0,
wantNameHits: 1,
},
{
name: "UUIDLikeNameFallback",
identifier: "a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6",
uuidEndpoint: &endpointResponse{
status: http.StatusNotFound,
errMsg: "Resource not found.",
},
nameEndpoint: &endpointResponse{
status: http.StatusOK,
},
expectedOwner: "me",
expectedName: "a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6",
wantUUIDHits: 1,
wantNameHits: 1,
},
{
name: "DashedUUIDNotFound",
identifier: "", // filled dynamically (standard dashed UUID)
uuidEndpoint: &endpointResponse{
status: http.StatusNotFound,
errMsg: "Resource not found.",
},
nameEndpoint: &endpointResponse{
status: http.StatusNotFound,
errMsg: "Resource not found.",
},
wantErr: true,
wantStatusCode: http.StatusNotFound,
// NameValid rejects dashed UUIDs (36 chars), so the
// name endpoint should not be called.
wantUUIDHits: 1,
wantNameHits: 0,
},
{
name: "NonNotFoundError",
identifier: "", // filled dynamically
uuidEndpoint: &endpointResponse{
status: http.StatusInternalServerError,
errMsg: "Internal server error.",
},
nameEndpoint: &endpointResponse{
status: http.StatusOK,
},
wantErr: true,
wantStatusCode: http.StatusInternalServerError,
wantUUIDHits: 1,
wantNameHits: 0,
},
{
name: "NameNotFound",
identifier: "nonexistent",
nameEndpoint: &endpointResponse{
status: http.StatusNotFound,
errMsg: "Resource not found.",
},
expectedOwner: "me",
expectedName: "nonexistent",
wantErr: true,
wantStatusCode: http.StatusNotFound,
wantUUIDHits: 0,
wantNameHits: 1,
},
{
name: "Forbidden",
identifier: "", // filled dynamically
uuidEndpoint: &endpointResponse{
status: http.StatusForbidden,
errMsg: "Forbidden.",
},
nameEndpoint: &endpointResponse{
status: http.StatusOK,
},
wantErr: true,
wantStatusCode: http.StatusForbidden,
wantUUIDHits: 1,
wantNameHits: 0,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
wsID := uuid.New()
expected := newWorkspace(wsID, "test-workspace")
// When identifier is empty, use the workspace UUID
// (standard dashed format).
identifier := tt.identifier
if identifier == "" {
identifier = wsID.String()
}
var uuidHits, nameHits atomic.Int64
r := chi.NewRouter()
if tt.uuidEndpoint != nil {
ep := tt.uuidEndpoint
// Use the expected workspace in OK responses
// unless the test overrides it.
if ep.status == http.StatusOK && ep.workspace.ID == uuid.Nil {
ep.workspace = expected
}
r.Get("/api/v2/workspaces/{workspace}", func(w http.ResponseWriter, req *http.Request) {
uuidHits.Add(1)
if ep.errMsg != "" {
writeJSON(w, ep.status, errResponse(ep.errMsg))
return
}
writeJSON(w, ep.status, ep.workspace)
})
}
if tt.nameEndpoint != nil {
ep := tt.nameEndpoint
if ep.status == http.StatusOK && ep.workspace.ID == uuid.Nil {
ep.workspace = expected
}
r.Get("/api/v2/users/{user}/workspace/{workspace}", func(w http.ResponseWriter, req *http.Request) {
nameHits.Add(1)
if tt.expectedOwner != "" {
assert.Equal(t, tt.expectedOwner, chi.URLParam(req, "user"))
}
if tt.expectedName != "" {
assert.Equal(t, tt.expectedName, chi.URLParam(req, "workspace"))
}
if ep.errMsg != "" {
writeJSON(w, ep.status, errResponse(ep.errMsg))
return
}
writeJSON(w, ep.status, ep.workspace)
})
}
srv := httptest.NewServer(r)
defer srv.Close()
u, err := url.Parse(srv.URL)
require.NoError(t, err)
client := codersdk.New(u)
ws, err := client.ResolveWorkspace(t.Context(), identifier)
if tt.wantErr {
require.Error(t, err)
if tt.wantStatusCode != 0 {
var sdkErr *codersdk.Error
require.ErrorAs(t, err, &sdkErr)
require.Equal(t, tt.wantStatusCode, sdkErr.StatusCode())
}
} else {
require.NoError(t, err)
require.Equal(t, expected.ID, ws.ID)
}
require.EqualValues(t, tt.wantUUIDHits, uuidHits.Load())
require.EqualValues(t, tt.wantNameHits, nameHits.Load())
})
}
// Cases that need a structurally different server setup.
t.Run("TransportError", func(t *testing.T) {
t.Parallel()
// Close the server immediately so the transport layer fails.
srv := httptest.NewServer(http.NotFoundHandler())
srvURL, err := url.Parse(srv.URL)
require.NoError(t, err)
srv.Close()
client := codersdk.New(srvURL)
_, err = client.ResolveWorkspace(t.Context(), uuid.NewString())
require.Error(t, err)
// Transport errors must not be swallowed by the 404
// fallback path. The error should NOT be a *codersdk.Error.
var sdkErr *codersdk.Error
require.False(t, errors.As(err, &sdkErr), "transport error should not be a codersdk.Error")
})
t.Run("InvalidIdentifier", func(t *testing.T) {
t.Parallel()
var hits atomic.Int64
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
hits.Add(1)
t.Errorf("unexpected HTTP request for invalid identifier: %s", req.URL.Path)
}))
defer srv.Close()
u, err := url.Parse(srv.URL)
require.NoError(t, err)
client := codersdk.New(u)
_, err = client.ResolveWorkspace(t.Context(), "a/b/c")
require.Error(t, err)
require.ErrorContains(t, err, "invalid workspace identifier: \"a/b/c\"")
require.EqualValues(t, 0, hits.Load(), "invalid identifiers should fail before any HTTP request")
})
}