From 04b309d3fc7042599a0e9a6e17cb35bf8c56c303 Mon Sep 17 00:00:00 2001 From: "tao.chen" <93983997+taochen-ct@users.noreply.github.com> Date: Thu, 2 Jul 2026 17:22:26 +0800 Subject: [PATCH] feat: harden workspace file APIs Co-Authored-By: Claude Fable 5 --- README.md | 126 ++- cmd/server/main.go | 13 +- configs/config.yaml | 2 + ...2026-07-02-workspace-file-api-hardening.md | 738 ++++++++++++++++++ internal/api/file_handler.go | 22 + internal/api/router.go | 2 + internal/api/router_test.go | 234 +++++- internal/api/workspace_handler.go | 15 + internal/fs/localfs.go | 5 +- internal/fs/localfs_test.go | 9 + internal/model/file.go | 10 + internal/model/workspace.go | 6 + internal/service/file_service.go | 18 +- internal/service/workspace_service.go | 5 + internal/workspace/manager.go | 28 + internal/workspace/manager_test.go | 51 ++ pkg/config/config.go | 18 + pkg/config/config_test.go | 36 + 18 files changed, 1262 insertions(+), 76 deletions(-) create mode 100644 docs/superpowers/plans/2026-07-02-workspace-file-api-hardening.md create mode 100644 internal/workspace/manager_test.go diff --git a/README.md b/README.md index 9dddf2f..4c44d66 100644 --- a/README.md +++ b/README.md @@ -1,18 +1,25 @@ # codespace -A runnable Go backend MVP skeleton that manages local workspaces, file operations through an abstract filesystem, and OpenCode processes. +A Go backend skeleton for managing local workspaces and file operations with process orchestration. ## V1 scope -- Workspace creation, lookup, and deletion backed by local directories. +- Workspace creation, listing, lookup, and deletion backed by local directories. - File read/write/list/mkdir/remove/rename/stat through a `FileSystem` interface with path-escape protection. +- Workspace root deletion protection — cannot remove the workspace root directory through the file API. +- Configurable per-file write size limit — oversized writes are rejected early before hitting disk. - OpenCode process start/stop/restart/status per workspace. - HTTP API using [Gin](https://github.com/gin-gonic/gin) for routing with JSON responses. - Explicit `http.Server` composition with configurable timeouts and max header bytes. +- Structured logging using Go standard library `log/slog` with JSON/text format and level control. -### Not implemented in v1 +### Safety -Docker, authentication, Git integration, LSP, AI Agent orchestration, web frontend, and full watcher/WebSocket event streaming are out of scope for the first version. +- File paths are workspace-relative; absolute paths are rejected. +- `..` escape attempts are rejected before filesystem operations. +- Deleting the workspace root through the file API is explicitly blocked. +- File content is never logged; structured logging only includes metadata. +- Workspace root paths are not exposed in API responses or logs. ## Run @@ -20,25 +27,7 @@ Docker, authentication, Git integration, LSP, AI Agent orchestration, web fronte make run ``` -The server listens on `:8080` by default. Override via `configs/config.yaml` or environment variables: - -| Variable | Description | -|---|---| -| `CODESPACE_ADDR` | Listen address | -| `CODESPACE_WORKSPACE_ROOT` | Workspace root directory | -| `CODESPACE_OPENCODE_COMMAND` | OpenCode binary path | -| `CODESPACE_READ_TIMEOUT` | HTTP read timeout (e.g. `15s`) | -| `CODESPACE_WRITE_TIMEOUT` | HTTP write timeout (e.g. `15s`) | -| `CODESPACE_IDLE_TIMEOUT` | HTTP idle timeout (e.g. `60s`) | -| `CODESPACE_MAX_HEADER_BYTES` | Max header bytes (e.g. `1048576`) | -| `CODESPACE_LOG_LEVEL` | Log level (`debug`, `info`, `warn`, `error`) | -| `CODESPACE_LOG_FORMAT` | Log format (`json`, `text`) | - -## Test - -```sh -make test -``` +The server listens on `:8080` by default. ## Configuration @@ -55,39 +44,80 @@ workspace: root: "./workspaces" process: opencodeCommand: "opencode" +file: + maxWriteBytes: 1048576 log: level: "info" format: "json" ``` -The server is started through an explicit `http.Server{Handler: router}` — it does not use `gin.Engine.Run()`. +### Environment overrides + +| Variable | Description | Default | +|---|---|---| +| `CODESPACE_ADDR` | Listen address | `:8080` | +| `CODESPACE_WORKSPACE_ROOT` | Workspace storage root | `./workspaces` | +| `CODESPACE_OPENCODE_COMMAND` | OpenCode binary path | `opencode` | +| `CODESPACE_FILE_MAX_WRITE_BYTES` | Max bytes per file write | `1048576` | +| `CODESPACE_READ_TIMEOUT` | HTTP read timeout | `15s` | +| `CODESPACE_WRITE_TIMEOUT` | HTTP write timeout | `15s` | +| `CODESPACE_IDLE_TIMEOUT` | HTTP idle timeout | `60s` | +| `CODESPACE_MAX_HEADER_BYTES` | Max HTTP header bytes | `1048576` | +| `CODESPACE_LOG_LEVEL` | Log level (`debug`, `info`, `warn`, `error`) | `info` | +| `CODESPACE_LOG_FORMAT` | Log format (`json`, `text`) | `json` | ## Logging -codespace uses the Go standard library `log/slog` for application logs. Default output is JSON to stdout. +codespace uses the Go standard library `log/slog` for application logs. +All runtime logging uses slog; `fmt`, `log.Printf`, and the Gin default logger are not used. -```yaml -log: - level: "info" - format: "json" -``` +## API -Environment overrides: +### Workspaces -- `CODESPACE_LOG_LEVEL` (`debug`, `info`, `warn`, `error`) -- `CODESPACE_LOG_FORMAT` (`json`, `text`) +| Method | Path | Description | +|---|---|---| +| `GET` | `/api/workspaces` | List all workspaces | +| `POST` | `/api/workspaces` | Create a workspace (`{"id": "name"}`) | +| `GET` | `/api/workspaces/:id` | Get workspace by ID | +| `DELETE` | `/api/workspaces/:id` | Delete workspace (stops running process first) | -All application logs must go through `log/slog`. Do not use `fmt.Print*`, `log.Printf`, `gin.Logger()`, or third-party logging libraries in server runtime code. +### Files -## API examples +| Method | Path | Description | +|---|---|---| +| `GET` | `/api/workspaces/:id/files?path=...` | List directory entries | +| `GET` | `/api/workspaces/:id/files/read?path=...` | Read file content | +| `GET` | `/api/workspaces/:id/files/stat?path=...` | Get file metadata (name, path, isDir, size, modTime) | +| `PUT` | `/api/workspaces/:id/files/write` | Write file (`{"path": "...", "content": "..."}`) | +| `POST` | `/api/workspaces/:id/files/mkdir` | Create directory (`{"path": "..."}`) | +| `DELETE` | `/api/workspaces/:id/files?path=...` | Remove file or directory recursively | +| `POST` | `/api/workspaces/:id/files/rename` | Rename file or directory (`{"oldPath": "...", "newPath": "..."}`) | -Health check: +### Process + +| Method | Path | Description | +|---|---|---| +| `POST` | `/api/workspaces/:id/process/start` | Start OpenCode process for workspace | +| `POST` | `/api/workspaces/:id/process/stop` | Stop OpenCode process | +| `POST` | `/api/workspaces/:id/process/restart` | Restart OpenCode process | +| `GET` | `/api/workspaces/:id/process/status` | Get process running status and PID | + +### Health + +| Method | Path | Description | +|---|---|---| +| `GET` | `/healthz` | Server health check | + +## Examples + +### Health check ```sh curl -s http://localhost:8080/healthz ``` -Create a workspace: +### Create a workspace ```sh curl -s -X POST http://localhost:8080/api/workspaces \ @@ -95,7 +125,13 @@ curl -s -X POST http://localhost:8080/api/workspaces \ -d '{"id":"user1"}' ``` -Write a file: +### List workspaces + +```sh +curl -s http://localhost:8080/api/workspaces +``` + +### Write a file ```sh curl -s -X PUT http://localhost:8080/api/workspaces/user1/files/write \ @@ -103,8 +139,20 @@ curl -s -X PUT http://localhost:8080/api/workspaces/user1/files/write \ -d '{"path":"main.go","content":"package main\n"}' ``` -Read a file: +### Read a file ```sh curl -s 'http://localhost:8080/api/workspaces/user1/files/read?path=main.go' ``` + +### Get file stat + +```sh +curl -s 'http://localhost:8080/api/workspaces/user1/files/stat?path=main.go' +``` + +## Test + +```sh +make test +``` diff --git a/cmd/server/main.go b/cmd/server/main.go index 9b04bf0..1162cd3 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -2,7 +2,7 @@ package main import ( "context" - "log" + "log/slog" "net/http" "os" "os/signal" @@ -18,21 +18,25 @@ import ( ) func main() { + startupLogger := slog.New(slog.NewJSONHandler(os.Stderr, nil)) + cfg, err := config.Load("configs/config.yaml") if err != nil { - log.Fatalf("failed to load config: %v", err) + startupLogger.Error("failed to load config", "error", err) + os.Exit(1) } lg, err := logger.New(cfg.Log) if err != nil { - log.Fatalf("failed to initialize logger: %v", err) + startupLogger.Error("failed to initialize logger", "error", err) + os.Exit(1) } workspaces := workspace.NewLocalManager(cfg.Workspace.Root) processes := process.NewManager(cfg.Process.OpenCodeCommand) workspaceSvc := service.NewWorkspaceService(workspaces, processes, lg) - fileSvc := service.NewFileService(workspaces) + fileSvc := service.NewFileService(workspaces, cfg.File.MaxWriteBytes) processSvc := service.NewProcessService(workspaces, processes, lg) router := api.NewRouter(workspaceSvc, fileSvc, processSvc, lg) @@ -48,7 +52,6 @@ func main() { lg.Info("server starting", "addr", cfg.Server.Addr, - "workspace_root", cfg.Workspace.Root, "read_timeout", cfg.Server.ReadTimeout.String(), "write_timeout", cfg.Server.WriteTimeout.String(), "idle_timeout", cfg.Server.IdleTimeout.String(), diff --git a/configs/config.yaml b/configs/config.yaml index a3694ce..b23f6e0 100644 --- a/configs/config.yaml +++ b/configs/config.yaml @@ -8,6 +8,8 @@ workspace: root: "./workspaces" process: opencodeCommand: "opencode" +file: + maxWriteBytes: 1048576 log: level: "info" format: "json" diff --git a/docs/superpowers/plans/2026-07-02-workspace-file-api-hardening.md b/docs/superpowers/plans/2026-07-02-workspace-file-api-hardening.md new file mode 100644 index 0000000..072431a --- /dev/null +++ b/docs/superpowers/plans/2026-07-02-workspace-file-api-hardening.md @@ -0,0 +1,738 @@ +# Workspace/File API Hardening Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Harden the Workspace/File API foundation before adding process sessions, watcher, frontend, Git, LSP, or agents. + +**Architecture:** Keep the current Gin + service + workspace/fs layering. Add missing workspace listing and file stat endpoints through the service layer, enforce file write size limits in `FileService`, prevent deleting the workspace root in `LocalFS`, and strengthen HTTP tests/error response coverage. + +**Tech Stack:** Go 1.25, Gin, standard library filesystem APIs, existing config/logger/service layers. + +## Global Constraints + +- Current branch starts from `main`; create a feature branch before code changes. +- All application logs must use `log/slog`; do not add `fmt.Print*`, `log.Printf`, `gin.Default`, or `gin.Logger`. +- Do not change existing API response shapes except where this plan explicitly adds new endpoints/responses. +- Do not bypass service layer from API handlers. +- Do not expose host absolute paths in API responses. +- Do not log file contents, request bodies, response bodies, environment variables, tokens, secrets, or workspace root in lifecycle/request logs. +- Do not implement Process Session, WebSocket watcher, frontend, Git, LSP, auth, CORS, or AI Agent features in this phase. +- Code generation/refactoring must be delegated to Codex MCP using exactly `model: "kimi/kimi-k2.7-code"`, `sandbox: "danger-full-access"`, and `approval-policy: "on-failure"`. +- Do not commit unless the user explicitly asks. + +--- + +## File Structure + +### Modify + +- `configs/config.yaml` — add `file.maxWriteBytes`. +- `pkg/config/config.go` — add `FileConfig`, defaults, YAML/env loading. +- `pkg/config/config_test.go` — add file config tests. +- `internal/workspace/manager.go` — add `List() ([]Workspace, error)` to manager interface/local implementation. +- `internal/workspace/path_test.go` or new `internal/workspace/manager_test.go` — add workspace listing tests. +- `internal/fs/localfs.go` — reject `Remove(".")` / empty path after clean as workspace-root deletion. +- `internal/fs/localfs_test.go` — add root delete rejection test. +- `internal/model/workspace.go` — add workspace list response if needed. +- `internal/model/file.go` — add stat response if needed. +- `internal/service/workspace_service.go` — add `List()`. +- `internal/service/file_service.go` — add max write bytes enforcement. +- `internal/api/router.go` — add `GET /api/workspaces` and `GET /api/workspaces/:id/files/stat`. +- `internal/api/workspace_handler.go` — add list handler. +- `internal/api/file_handler.go` — add stat handler and preserve existing handlers. +- `internal/api/router_test.go` — add complete workspace/file/error HTTP tests. +- `cmd/server/main.go` — pass file max write bytes into `FileService`. +- `README.md` — document Phase 1 API/config behavior. + +--- + +## Task 1: Add workspace list endpoint + +**Files:** +- Modify: `internal/workspace/manager.go` +- Create or Modify: `internal/workspace/manager_test.go` +- Modify: `internal/service/workspace_service.go` +- Modify: `internal/model/workspace.go` +- Modify: `internal/api/router.go` +- Modify: `internal/api/workspace_handler.go` +- Modify: `internal/api/router_test.go` + +**Interfaces:** +- Produces: `workspace.Manager.List() ([]Workspace, error)`. +- Produces: `(*service.WorkspaceService).List() ([]workspace.Workspace, error)`. +- Produces: `GET /api/workspaces` returning JSON array of objects with `id` only. + +- [ ] **Step 1: Add failing workspace list tests** + +Add tests in `internal/workspace/manager_test.go`: + +```go +func TestLocalManagerListReturnsSortedWorkspaces(t *testing.T) { + mgr := NewLocalManager(t.TempDir()) + if _, err := mgr.Create("beta"); err != nil { t.Fatal(err) } + if _, err := mgr.Create("alpha"); err != nil { t.Fatal(err) } + + got, err := mgr.List() + if err != nil { t.Fatal(err) } + if len(got) != 2 { t.Fatalf("len = %d, want 2", len(got)) } + if got[0].ID != "alpha" || got[1].ID != "beta" { + t.Fatalf("ids = %q, %q; want alpha, beta", got[0].ID, got[1].ID) + } + if got[0].Root == "" || got[1].Root == "" { + t.Fatal("manager list should keep internal roots for service use") + } +} + +func TestLocalManagerListSkipsUnsafeDirectoryNames(t *testing.T) { + root := t.TempDir() + if err := os.Mkdir(filepath.Join(root, "valid"), 0o755); err != nil { t.Fatal(err) } + if err := os.Mkdir(filepath.Join(root, "bad name"), 0o755); err != nil { t.Fatal(err) } + + mgr := NewLocalManager(root) + got, err := mgr.List() + if err != nil { t.Fatal(err) } + if len(got) != 1 || got[0].ID != "valid" { + t.Fatalf("got %#v, want only valid", got) + } +} +``` + +Imports required: `os`, `path/filepath`, `testing`. + +- [ ] **Step 2: Implement manager/service list** + +Update `internal/workspace/manager.go`: + +```go +type Manager interface { + Create(id string) (*Workspace, error) + Get(id string) (*Workspace, error) + Delete(id string) error + List() ([]Workspace, error) +} +``` + +`LocalManager.List` behavior: + +- Ensure base workspace root exists with `os.MkdirAll(m.root, 0o755)`. +- Read direct children with `os.ReadDir(m.root)`. +- Include only directories whose names pass `ValidID`. +- Return sorted by `ID`. +- Return `[]Workspace` with internal `Root` populated. + +Update `WorkspaceService`: + +```go +func (s *WorkspaceService) List() ([]workspace.Workspace, error) { + return s.workspaces.List() +} +``` + +- [ ] **Step 3: Add model/API list response** + +Update `internal/model/workspace.go` if needed: + +```go +type WorkspaceListResponse struct { + Workspaces []WorkspaceResponse `json:"workspaces"` +} +``` + +Add route in `internal/api/router.go`: + +```go +api.GET("/workspaces", wsHandler.list) +``` + +Add handler in `internal/api/workspace_handler.go`: + +```go +func (h *workspaceHandler) list(c *gin.Context) { + workspaces, err := h.svc.List() + if err != nil { writeError(c, err); return } + resp := model.WorkspaceListResponse{Workspaces: make([]model.WorkspaceResponse, 0, len(workspaces))} + for _, ws := range workspaces { + resp.Workspaces = append(resp.Workspaces, model.WorkspaceResponse{ID: ws.ID}) + } + c.JSON(http.StatusOK, resp) +} +``` + +Do not expose `Workspace.Root`. + +- [ ] **Step 4: Add HTTP list test** + +Add to `internal/api/router_test.go`: + +```go +func TestListWorkspaces(t *testing.T) { + router := setupTestRouter(t) + for _, id := range []string{"beta", "alpha"} { + payload, _ := json.Marshal(map[string]string{"id": id}) + req := httptest.NewRequest("POST", "/api/workspaces", bytes.NewReader(payload)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + if rec.Code != http.StatusCreated { t.Fatalf("create %s: %d %s", id, rec.Code, rec.Body.String()) } + } + + req := httptest.NewRequest("GET", "/api/workspaces", nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) } + + var resp struct { Workspaces []struct{ ID string `json:"id"` } `json:"workspaces"` } + if err := json.NewDecoder(rec.Body).Decode(&resp); err != nil { t.Fatal(err) } + if len(resp.Workspaces) != 2 || resp.Workspaces[0].ID != "alpha" || resp.Workspaces[1].ID != "beta" { + t.Fatalf("response = %#v", resp) + } +} +``` + +- [ ] **Step 5: Verify task** + +Run: + +```sh +gofmt -w internal/workspace internal/service internal/model internal/api +go test ./internal/workspace ./internal/api +``` + +Expected: all tests pass. + +--- + +## Task 2: Harden LocalFS remove and expose file stat + +**Files:** +- Modify: `internal/fs/localfs.go` +- Modify: `internal/fs/localfs_test.go` +- Modify: `internal/model/file.go` +- Modify: `internal/api/router.go` +- Modify: `internal/api/file_handler.go` +- Modify: `internal/api/router_test.go` + +**Interfaces:** +- Produces: `LocalFS.Remove(".")` and `Remove("")` return bad request. +- Produces: `GET /api/workspaces/:id/files/stat?path=...` returning file metadata. + +- [ ] **Step 1: Add failing root delete test** + +Add to `internal/fs/localfs_test.go`: + +```go +func TestLocalFSRejectsRemoveRoot(t *testing.T) { + fsys := NewLocal(t.TempDir()) + for _, path := range []string{"", "."} { + if err := fsys.Remove(path); err == nil { + t.Fatalf("Remove(%q) expected error", path) + } + } +} +``` + +- [ ] **Step 2: Implement root delete protection** + +Update `LocalFS.Remove` after `resolve`: + +```go +if rel == "." { + return util.New(util.CodeBadRequest, "cannot remove workspace root") +} +``` + +This requires keeping the `rel` return from `resolve`. + +- [ ] **Step 3: Add stat response model and handler** + +Update `internal/model/file.go`: + +```go +type FileInfoResponse struct { + Name string `json:"name"` + Path string `json:"path"` + IsDir bool `json:"isDir"` + Size int64 `json:"size"` + ModTime string `json:"modTime"` +} +``` + +Add route: + +```go +api.GET("/workspaces/:id/files/stat", fileHandler.stat) +``` + +Add handler: + +```go +func (h *fileHandler) stat(c *gin.Context) { + id := c.Param("id") + path := c.Query("path") + if path == "" { writeBadRequest(c, "path is required"); return } + info, err := h.svc.Stat(id, path) + if err != nil { writeError(c, err); return } + c.JSON(http.StatusOK, model.FileInfoResponse{ + Name: info.Name, + Path: info.Path, + IsDir: info.IsDir, + Size: info.Size, + ModTime: info.ModTime.Format(time.RFC3339Nano), + }) +} +``` + +Add `time` import to file handler. + +- [ ] **Step 4: Add HTTP stat/root delete tests** + +Add tests to `internal/api/router_test.go`: + +```go +func TestFileStat(t *testing.T) { + router := setupTestRouter(t) + createWorkspaceForTest(t, router, "user1") + writeFileForTest(t, router, "user1", "main.go", "package main\n") + + req := httptest.NewRequest("GET", "/api/workspaces/user1/files/stat?path=main.go", nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) } + var resp struct { Name string `json:"name"`; Path string `json:"path"`; IsDir bool `json:"isDir"`; Size int64 `json:"size"` } + if err := json.NewDecoder(rec.Body).Decode(&resp); err != nil { t.Fatal(err) } + if resp.Name != "main.go" || resp.Path != "main.go" || resp.IsDir || resp.Size != int64(len("package main\n")) { + t.Fatalf("resp=%#v", resp) + } +} + +func TestRemoveWorkspaceRootIsRejected(t *testing.T) { + router := setupTestRouter(t) + createWorkspaceForTest(t, router, "user1") + req := httptest.NewRequest("DELETE", "/api/workspaces/user1/files?path=.", nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + if rec.Code != http.StatusBadRequest { t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) } +} +``` + +If helper functions do not exist, create these in `router_test.go`: + +```go +func createWorkspaceForTest(t *testing.T, router http.Handler, id string) { ... } +func writeFileForTest(t *testing.T, router http.Handler, id, path, content string) { ... } +``` + +- [ ] **Step 5: Verify task** + +Run: + +```sh +gofmt -w internal/fs internal/model internal/api +go test ./internal/fs ./internal/api +``` + +Expected: all tests pass. + +--- + +## Task 3: Add max write size limit + +**Files:** +- Modify: `configs/config.yaml` +- Modify: `pkg/config/config.go` +- Modify: `pkg/config/config_test.go` +- Modify: `internal/service/file_service.go` +- Modify: `cmd/server/main.go` +- Modify: `internal/api/router_test.go` + +**Interfaces:** +- Produces: `config.FileConfig{MaxWriteBytes int64}`. +- Produces: `service.NewFileService(workspaces workspace.Manager, maxWriteBytes int64) *FileService`. +- Enforces oversized writes return `400 bad_request` before hitting filesystem. + +- [ ] **Step 1: Add config tests** + +Add to `pkg/config/config_test.go`: + +```go +func TestDefaultIncludesFileConfig(t *testing.T) { + cfg := Default() + if cfg.File.MaxWriteBytes != 1048576 { + t.Fatalf("File.MaxWriteBytes = %d, want 1048576", cfg.File.MaxWriteBytes) + } +} + +func TestLoadParsesFileConfigFromYAML(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + data := []byte(`file: + maxWriteBytes: 128 +`) + if err := os.WriteFile(path, data, 0o644); err != nil { t.Fatal(err) } + cfg, err := Load(path) + if err != nil { t.Fatal(err) } + if cfg.File.MaxWriteBytes != 128 { t.Fatalf("got %d", cfg.File.MaxWriteBytes) } +} + +func TestLoadAppliesFileEnvironmentOverride(t *testing.T) { + t.Setenv("CODESPACE_FILE_MAX_WRITE_BYTES", "256") + cfg, err := Load(filepath.Join(t.TempDir(), "missing.yaml")) + if err != nil { t.Fatal(err) } + if cfg.File.MaxWriteBytes != 256 { t.Fatalf("got %d", cfg.File.MaxWriteBytes) } +} +``` + +- [ ] **Step 2: Implement file config** + +Update `pkg/config/config.go`: + +```go +type Config struct { ... File FileConfig `yaml:"file"` } +type FileConfig struct { MaxWriteBytes int64 `yaml:"maxWriteBytes"` } +``` + +Update raw config to include `File FileConfig`. + +Default: + +```go +File: FileConfig{MaxWriteBytes: 1 << 20}, +``` + +Raw/env handling: + +```go +if raw.File.MaxWriteBytes != 0 { cfg.File.MaxWriteBytes = raw.File.MaxWriteBytes } +if v := os.Getenv("CODESPACE_FILE_MAX_WRITE_BYTES"); v != "" { parse int64 and assign } +``` + +Invalid env integer returns error. + +Update `configs/config.yaml`: + +```yaml +file: + maxWriteBytes: 1048576 +``` + +- [ ] **Step 3: Enforce in FileService** + +Update `internal/service/file_service.go`: + +```go +type FileService struct { + workspaces workspace.Manager + maxWriteBytes int64 +} + +func NewFileService(workspaces workspace.Manager, maxWriteBytes int64) *FileService { + if maxWriteBytes <= 0 { maxWriteBytes = 1 << 20 } + return &FileService{workspaces: workspaces, maxWriteBytes: maxWriteBytes} +} +``` + +In `Write`: + +```go +if int64(len(data)) > s.maxWriteBytes { + return util.New(util.CodeBadRequest, "file exceeds max write size") +} +``` + +Do not log file content. + +Update call sites: + +```go +fileSvc := service.NewFileService(workspaces, cfg.File.MaxWriteBytes) +``` + +Tests can use `service.NewFileService(wsMgr, 0)` for default. + +- [ ] **Step 4: Add HTTP oversized write test** + +Add to `internal/api/router_test.go` a setup helper that accepts max write bytes: + +```go +func setupTestRouterWithMaxWriteBytes(t *testing.T, maxWriteBytes int64) http.Handler { ... } +``` + +Use it in: + +```go +func TestWriteFileRejectsOversizedContent(t *testing.T) { + router := setupTestRouterWithMaxWriteBytes(t, 4) + createWorkspaceForTest(t, router, "user1") + payload, _ := json.Marshal(map[string]string{"path": "big.txt", "content": "12345"}) + req := httptest.NewRequest("PUT", "/api/workspaces/user1/files/write", bytes.NewReader(payload)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + if rec.Code != http.StatusBadRequest { t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) } +} +``` + +- [ ] **Step 5: Verify task** + +Run: + +```sh +gofmt -w cmd internal pkg configs +go test ./pkg/config ./internal/api ./internal/service +``` + +Expected: all tests pass. + +--- + +## Task 4: Complete file handler and error response tests + +**Files:** +- Modify: `internal/api/router_test.go` + +**Interfaces:** +- Produces HTTP tests for list/read/write/mkdir/remove/rename/stat and consistent error JSON. + +- [ ] **Step 1: Add full file operation test** + +Add or refactor `internal/api/router_test.go` to include: + +```go +func TestFileLifecycle(t *testing.T) { + router := setupTestRouter(t) + createWorkspaceForTest(t, router, "user1") + + mkdirPayload, _ := json.Marshal(map[string]string{"path": "dir"}) + req := httptest.NewRequest("POST", "/api/workspaces/user1/files/mkdir", bytes.NewReader(mkdirPayload)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + if rec.Code != http.StatusNoContent { t.Fatalf("mkdir status=%d body=%s", rec.Code, rec.Body.String()) } + + writeFileForTest(t, router, "user1", "dir/old.txt", "hello") + + req = httptest.NewRequest("GET", "/api/workspaces/user1/files?path=dir", nil) + rec = httptest.NewRecorder() + router.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { t.Fatalf("list status=%d body=%s", rec.Code, rec.Body.String()) } + + renamePayload, _ := json.Marshal(map[string]string{"oldPath": "dir/old.txt", "newPath": "dir/new.txt"}) + req = httptest.NewRequest("POST", "/api/workspaces/user1/files/rename", bytes.NewReader(renamePayload)) + req.Header.Set("Content-Type", "application/json") + rec = httptest.NewRecorder() + router.ServeHTTP(rec, req) + if rec.Code != http.StatusNoContent { t.Fatalf("rename status=%d body=%s", rec.Code, rec.Body.String()) } + + req = httptest.NewRequest("GET", "/api/workspaces/user1/files/read?path=dir/new.txt", nil) + rec = httptest.NewRecorder() + router.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { t.Fatalf("read status=%d body=%s", rec.Code, rec.Body.String()) } + + req = httptest.NewRequest("DELETE", "/api/workspaces/user1/files?path=dir/new.txt", nil) + rec = httptest.NewRecorder() + router.ServeHTTP(rec, req) + if rec.Code != http.StatusNoContent { t.Fatalf("remove status=%d body=%s", rec.Code, rec.Body.String()) } +} +``` + +- [ ] **Step 2: Add consistent error JSON tests** + +Add helper: + +```go +func assertErrorResponse(t *testing.T, rec *httptest.ResponseRecorder, status int, code string) { + t.Helper() + if rec.Code != status { t.Fatalf("status=%d want=%d body=%s", rec.Code, status, rec.Body.String()) } + var body struct { Error struct { Code string `json:"code"`; Message string `json:"message"` } `json:"error"` } + if err := json.NewDecoder(rec.Body).Decode(&body); err != nil { t.Fatal(err) } + if body.Error.Code != code { t.Fatalf("code=%q want=%q", body.Error.Code, code) } + if body.Error.Message == "" { t.Fatal("error message is empty") } +} +``` + +Add tests: + +```go +func TestErrorResponsesAreConsistent(t *testing.T) { + router := setupTestRouter(t) + + req := httptest.NewRequest("GET", "/api/workspaces/missing", nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + assertErrorResponse(t, rec, http.StatusNotFound, "not_found") + + req = httptest.NewRequest("POST", "/api/workspaces", bytes.NewBufferString("{")) + req.Header.Set("Content-Type", "application/json") + rec = httptest.NewRecorder() + router.ServeHTTP(rec, req) + assertErrorResponse(t, rec, http.StatusBadRequest, "bad_request") + + createWorkspaceForTest(t, router, "user1") + req = httptest.NewRequest("GET", "/api/workspaces/user1/files/read?path=../secret", nil) + rec = httptest.NewRecorder() + router.ServeHTTP(rec, req) + assertErrorResponse(t, rec, http.StatusBadRequest, "bad_request") +} +``` + +- [ ] **Step 3: Verify task** + +Run: + +```sh +gofmt -w internal/api +go test ./internal/api +``` + +Expected: all API tests pass. + +--- + +## Task 5: README and final validation + +**Files:** +- Modify: `README.md` + +**Interfaces:** +- Produces documented workspace/file endpoints and file write limit config. + +- [ ] **Step 1: Update README API docs** + +Add documentation for: + +```text +GET /api/workspaces +POST /api/workspaces +GET /api/workspaces/:id +DELETE /api/workspaces/:id +GET /api/workspaces/:id/files?path=. +GET /api/workspaces/:id/files/read?path=main.go +GET /api/workspaces/:id/files/stat?path=main.go +PUT /api/workspaces/:id/files/write +POST /api/workspaces/:id/files/mkdir +DELETE /api/workspaces/:id/files?path=main.go +POST /api/workspaces/:id/files/rename +``` + +Document config: + +```yaml +file: + maxWriteBytes: 1048576 +``` + +Document env override: + +```text +CODESPACE_FILE_MAX_WRITE_BYTES +``` + +Document safety rules: + +- File paths are workspace-relative. +- Absolute paths and `..` escapes are rejected. +- Deleting workspace root through file API is rejected. +- File content is not logged. + +- [ ] **Step 2: Final validation** + +Run: + +```sh +gofmt -w cmd internal pkg +go mod tidy +go test ./... +grep -R "log\.Printf\|log\.Println\|log\.Fatal\|fmt\.Print\|gin\.Default\|gin\.Logger" cmd internal pkg || true +``` + +Expected: + +- Tests pass. +- slog-only grep only shows allowed pre-slog `log.Fatalf` in `cmd/server/main.go`, if present. + +- [ ] **Step 3: Server smoke test** + +Run: + +```sh +(go run ./cmd/server > /tmp/codespace-server.log 2>&1 & pid=$!; \ + for i in $(seq 1 30); do \ + if curl -fsS http://localhost:8080/healthz; then \ + kill $pid; wait $pid 2>/dev/null || true; exit 0; \ + fi; \ + sleep 0.2; \ + done; \ + kill $pid 2>/dev/null || true; \ + wait $pid 2>/dev/null || true; \ + cat /tmp/codespace-server.log; exit 1) +``` + +Expected: + +```json +{"status":"ok"} +``` + +- [ ] **Step 4: Diff scope check** + +Run: + +```sh +git diff --stat +git status --short +``` + +Expected: + +- Changes limited to workspace/file API hardening, tests, config, docs/plan, and README. +- No Process Session, WebSocket watcher, frontend, Git, LSP, auth, CORS, or Agent implementation. + +--- + +## Codex Execution Prompt + +Use this exact structure when dispatching Codex: + +```text +## Context +- Project root: /Users/taochen/llm/codespace +- Current branch: feat/workspace-file-api-hardening +- Go baseline: 1.25 +- Existing backend uses Gin, explicit http.Server, slog logging, service layer, LocalFS. +- Implementation plan: docs/superpowers/plans/2026-07-02-workspace-file-api-hardening.md + +## Task +Implement Phase 1 Workspace/File API hardening. +Steps: +1. Add workspace list support through workspace.Manager, WorkspaceService, and GET /api/workspaces. +2. Prevent deleting workspace root through LocalFS.Remove and file API. +3. Add file stat endpoint GET /api/workspaces/:id/files/stat?path=... +4. Add file.maxWriteBytes config and enforce it in FileService.Write. +5. Add complete file handler tests and consistent error response tests. +6. Update README API/config docs. +7. Run final validation. + +## Constraints +- All application logs must use slog; do not add fmt.Print*, log.Printf/log.Println, gin.Default, or gin.Logger. +- Do not bypass service layer from API handlers. +- Do not expose host absolute paths in API responses. +- Do not log file contents, request bodies, response bodies, env, tokens, secrets, or workspace root in lifecycle/request logs. +- Do not implement Process Session, WebSocket watcher, frontend, Git, LSP, auth, CORS, or AI Agent features. +- Do not commit. + +## Acceptance +- gofmt -w cmd internal pkg succeeds. +- go mod tidy succeeds. +- go test ./... passes. +- slog-only grep has no disallowed runtime logging. +- go run ./cmd/server starts and /healthz returns {"status":"ok"}. +``` + +--- + +## Self-Review + +- Spec coverage: This plan covers workspace listing, file stat, root delete protection, write size config, complete file handler tests, error response tests, README docs, and validation. +- Placeholder scan: No TBD/TODO/ambiguous implementation steps remain. +- Type consistency: New `List`, `Stat`, `MaxWriteBytes`, and constructor signatures are consistent across manager/service/api/main/test steps. +- Scope check: Plan is limited to Workspace/File API hardening. It does not implement Process Session, WebSocket watcher, frontend, Git, LSP, auth, CORS, or Agent features. diff --git a/internal/api/file_handler.go b/internal/api/file_handler.go index efaa1ea..4fc8e13 100644 --- a/internal/api/file_handler.go +++ b/internal/api/file_handler.go @@ -2,6 +2,7 @@ package api import ( "net/http" + "time" "codespace/internal/model" "codespace/internal/service" @@ -112,3 +113,24 @@ func (h *fileHandler) rename(c *gin.Context) { } c.Status(http.StatusNoContent) } + +func (h *fileHandler) stat(c *gin.Context) { + id := c.Param("id") + path := c.Query("path") + if path == "" { + writeBadRequest(c, "path is required") + return + } + info, err := h.svc.Stat(id, path) + if err != nil { + writeError(c, err) + return + } + c.JSON(http.StatusOK, model.FileInfoResponse{ + Name: info.Name, + Path: info.Path, + IsDir: info.IsDir, + Size: info.Size, + ModTime: info.ModTime.Format(time.RFC3339Nano), + }) +} diff --git a/internal/api/router.go b/internal/api/router.go index 8dc5a4d..6ab780f 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -23,12 +23,14 @@ func NewRouter(workspaces *service.WorkspaceService, files *service.FileService, r.GET("/healthz", healthHandler) api := r.Group("/api") + api.GET("/workspaces", wsHandler.list) api.POST("/workspaces", wsHandler.create) api.GET("/workspaces/:id", wsHandler.get) api.DELETE("/workspaces/:id", wsHandler.delete) api.GET("/workspaces/:id/files", fileHandler.list) api.GET("/workspaces/:id/files/read", fileHandler.read) + api.GET("/workspaces/:id/files/stat", fileHandler.stat) api.PUT("/workspaces/:id/files/write", fileHandler.write) api.POST("/workspaces/:id/files/mkdir", fileHandler.mkdir) api.DELETE("/workspaces/:id/files", fileHandler.remove) diff --git a/internal/api/router_test.go b/internal/api/router_test.go index 737409f..1486640 100644 --- a/internal/api/router_test.go +++ b/internal/api/router_test.go @@ -13,16 +13,67 @@ import ( ) func setupTestRouter(t *testing.T) http.Handler { + t.Helper() + return setupTestRouterWithMaxWriteBytes(t, 1<<20) +} + +func setupTestRouterWithMaxWriteBytes(t *testing.T, maxWriteBytes int64) http.Handler { t.Helper() root := t.TempDir() wsMgr := workspace.NewLocalManager(root) procMgr := process.NewManager("") wsSvc := service.NewWorkspaceService(wsMgr, procMgr, nil) - fileSvc := service.NewFileService(wsMgr) + fileSvc := service.NewFileService(wsMgr, maxWriteBytes) procSvc := service.NewProcessService(wsMgr, procMgr, nil) return NewRouter(wsSvc, fileSvc, procSvc, nil) } +func createWorkspaceForTest(t *testing.T, router http.Handler, id string) { + t.Helper() + payload, _ := json.Marshal(map[string]string{"id": id}) + req := httptest.NewRequest("POST", "/api/workspaces", bytes.NewReader(payload)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + if rec.Code != http.StatusCreated { + t.Fatalf("create workspace %q: status %d, body %s", id, rec.Code, rec.Body.String()) + } +} + +func writeFileForTest(t *testing.T, router http.Handler, id, path, content string) { + t.Helper() + payload, _ := json.Marshal(map[string]string{"path": path, "content": content}) + req := httptest.NewRequest("PUT", "/api/workspaces/"+id+"/files/write", bytes.NewReader(payload)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + if rec.Code != http.StatusNoContent { + t.Fatalf("write %q: status %d, body %s", path, rec.Code, rec.Body.String()) + } +} + +func assertErrorResponse(t *testing.T, rec *httptest.ResponseRecorder, status int, code string) { + t.Helper() + if rec.Code != status { + t.Fatalf("status = %d, want %d; body %s", rec.Code, status, rec.Body.String()) + } + var body struct { + Error struct { + Code string `json:"code"` + Message string `json:"message"` + } `json:"error"` + } + if err := json.NewDecoder(rec.Body).Decode(&body); err != nil { + t.Fatal(err) + } + if body.Error.Code != code { + t.Fatalf("code = %q, want %q", body.Error.Code, code) + } + if body.Error.Message == "" { + t.Fatal("error message is empty") + } +} + func TestHealthz(t *testing.T) { router := setupTestRouter(t) req := httptest.NewRequest("GET", "/healthz", nil) @@ -64,32 +115,47 @@ func TestCreateWorkspace(t *testing.T) { } } -func TestWriteReadFile(t *testing.T) { +func TestListWorkspaces(t *testing.T) { router := setupTestRouter(t) + for _, id := range []string{"beta", "alpha"} { + payload, _ := json.Marshal(map[string]string{"id": id}) + req := httptest.NewRequest("POST", "/api/workspaces", bytes.NewReader(payload)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + if rec.Code != http.StatusCreated { + t.Fatalf("create %s: %d %s", id, rec.Code, rec.Body.String()) + } + } - // Create workspace first. - payload, _ := json.Marshal(map[string]string{"id": "user1"}) - req := httptest.NewRequest("POST", "/api/workspaces", bytes.NewReader(payload)) - req.Header.Set("Content-Type", "application/json") + req := httptest.NewRequest("GET", "/api/workspaces", nil) rec := httptest.NewRecorder() router.ServeHTTP(rec, req) - if rec.Code != http.StatusCreated { - t.Fatalf("create workspace: status=%d body=%s", rec.Code, rec.Body.String()) + if rec.Code != http.StatusOK { + t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) } - // Write file. - writePayload, _ := json.Marshal(map[string]string{"path": "main.go", "content": "package main\n"}) - req = httptest.NewRequest("PUT", "/api/workspaces/user1/files/write", bytes.NewReader(writePayload)) - req.Header.Set("Content-Type", "application/json") - rec = httptest.NewRecorder() - router.ServeHTTP(rec, req) - if rec.Code != http.StatusNoContent { - t.Fatalf("write file: status=%d want=%d body=%s", rec.Code, http.StatusNoContent, rec.Body.String()) + var resp struct { + Workspaces []struct { + ID string `json:"id"` + } `json:"workspaces"` } + if err := json.NewDecoder(rec.Body).Decode(&resp); err != nil { + t.Fatal(err) + } + if len(resp.Workspaces) != 2 || resp.Workspaces[0].ID != "alpha" || resp.Workspaces[1].ID != "beta" { + t.Fatalf("response = %#v", resp) + } +} - // Read file. - req = httptest.NewRequest("GET", "/api/workspaces/user1/files/read?path=main.go", nil) - rec = httptest.NewRecorder() +func TestWriteReadFile(t *testing.T) { + router := setupTestRouter(t) + createWorkspaceForTest(t, router, "user1") + writeFileForTest(t, router, "user1", "main.go", "package main\n") + + // Read file + req := httptest.NewRequest("GET", "/api/workspaces/user1/files/read?path=main.go", nil) + rec := httptest.NewRecorder() router.ServeHTTP(rec, req) if rec.Code != http.StatusOK { t.Fatalf("read file: status=%d want=%d body=%s", rec.Code, http.StatusOK, rec.Body.String()) @@ -104,23 +170,137 @@ func TestWriteReadFile(t *testing.T) { } } -func TestProcessStatus(t *testing.T) { +func TestFileStat(t *testing.T) { router := setupTestRouter(t) + createWorkspaceForTest(t, router, "user1") + writeFileForTest(t, router, "user1", "main.go", "package main\n") - // Create workspace. - payload, _ := json.Marshal(map[string]string{"id": "user1"}) - req := httptest.NewRequest("POST", "/api/workspaces", bytes.NewReader(payload)) + req := httptest.NewRequest("GET", "/api/workspaces/user1/files/stat?path=main.go", nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) + } + var resp struct { + Name string `json:"name"` + Path string `json:"path"` + IsDir bool `json:"isDir"` + Size int64 `json:"size"` + } + if err := json.NewDecoder(rec.Body).Decode(&resp); err != nil { + t.Fatal(err) + } + if resp.Name != "main.go" || resp.Path != "main.go" || resp.IsDir || resp.Size != int64(len("package main\n")) { + t.Fatalf("resp=%#v", resp) + } +} + +func TestRemoveWorkspaceRootIsRejected(t *testing.T) { + router := setupTestRouter(t) + createWorkspaceForTest(t, router, "user1") + req := httptest.NewRequest("DELETE", "/api/workspaces/user1/files?path=.", nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + if rec.Code != http.StatusBadRequest { + t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) + } +} + +func TestWriteFileRejectsOversizedContent(t *testing.T) { + router := setupTestRouterWithMaxWriteBytes(t, 4) + createWorkspaceForTest(t, router, "user1") + // Content length 5 exceeds maxWriteBytes of 4 + payload, _ := json.Marshal(map[string]string{"path": "big.txt", "content": "12345"}) + req := httptest.NewRequest("PUT", "/api/workspaces/user1/files/write", bytes.NewReader(payload)) req.Header.Set("Content-Type", "application/json") rec := httptest.NewRecorder() router.ServeHTTP(rec, req) - if rec.Code != http.StatusCreated { - t.Fatalf("create workspace: status=%d", rec.Code) + assertErrorResponse(t, rec, http.StatusBadRequest, "bad_request") +} + +func TestFileLifecycle(t *testing.T) { + router := setupTestRouter(t) + createWorkspaceForTest(t, router, "user1") + + // Mkdir + mkdirPayload, _ := json.Marshal(map[string]string{"path": "dir"}) + req := httptest.NewRequest("POST", "/api/workspaces/user1/files/mkdir", bytes.NewReader(mkdirPayload)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + if rec.Code != http.StatusNoContent { + t.Fatalf("mkdir status=%d body=%s", rec.Code, rec.Body.String()) } - // Get process status. - req = httptest.NewRequest("GET", "/api/workspaces/user1/process/status", nil) + // Write + writeFileForTest(t, router, "user1", "dir/old.txt", "hello") + + // List + req = httptest.NewRequest("GET", "/api/workspaces/user1/files?path=dir", nil) rec = httptest.NewRecorder() router.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("list status=%d body=%s", rec.Code, rec.Body.String()) + } + + // Rename + renamePayload, _ := json.Marshal(map[string]string{"oldPath": "dir/old.txt", "newPath": "dir/new.txt"}) + req = httptest.NewRequest("POST", "/api/workspaces/user1/files/rename", bytes.NewReader(renamePayload)) + req.Header.Set("Content-Type", "application/json") + rec = httptest.NewRecorder() + router.ServeHTTP(rec, req) + if rec.Code != http.StatusNoContent { + t.Fatalf("rename status=%d body=%s", rec.Code, rec.Body.String()) + } + + // Read new location + req = httptest.NewRequest("GET", "/api/workspaces/user1/files/read?path=dir/new.txt", nil) + rec = httptest.NewRecorder() + router.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("read status=%d body=%s", rec.Code, rec.Body.String()) + } + + // Remove + req = httptest.NewRequest("DELETE", "/api/workspaces/user1/files?path=dir/new.txt", nil) + rec = httptest.NewRecorder() + router.ServeHTTP(rec, req) + if rec.Code != http.StatusNoContent { + t.Fatalf("remove status=%d body=%s", rec.Code, rec.Body.String()) + } +} + +func TestErrorResponsesAreConsistent(t *testing.T) { + router := setupTestRouter(t) + + // Not found workspace + req := httptest.NewRequest("GET", "/api/workspaces/missing", nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + assertErrorResponse(t, rec, http.StatusNotFound, "not_found") + + // Invalid JSON + req = httptest.NewRequest("POST", "/api/workspaces", bytes.NewBufferString("{")) + req.Header.Set("Content-Type", "application/json") + rec = httptest.NewRecorder() + router.ServeHTTP(rec, req) + assertErrorResponse(t, rec, http.StatusBadRequest, "bad_request") + + // Path escape attempt + createWorkspaceForTest(t, router, "user1") + req = httptest.NewRequest("GET", "/api/workspaces/user1/files/read?path=../secret", nil) + rec = httptest.NewRecorder() + router.ServeHTTP(rec, req) + assertErrorResponse(t, rec, http.StatusBadRequest, "bad_request") +} + +func TestProcessStatus(t *testing.T) { + router := setupTestRouter(t) + createWorkspaceForTest(t, router, "user1") + + req := httptest.NewRequest("GET", "/api/workspaces/user1/process/status", nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) if rec.Code != http.StatusOK { t.Fatalf("status = %d, want %d; body=%s", rec.Code, http.StatusOK, rec.Body.String()) diff --git a/internal/api/workspace_handler.go b/internal/api/workspace_handler.go index 124c891..807024d 100644 --- a/internal/api/workspace_handler.go +++ b/internal/api/workspace_handler.go @@ -31,6 +31,21 @@ func (h *workspaceHandler) create(c *gin.Context) { c.JSON(http.StatusCreated, model.WorkspaceResponse{ID: ws.ID}) } +func (h *workspaceHandler) list(c *gin.Context) { + workspaces, err := h.svc.List() + if err != nil { + writeError(c, err) + return + } + resp := model.WorkspaceListResponse{ + Workspaces: make([]model.WorkspaceResponse, 0, len(workspaces)), + } + for _, ws := range workspaces { + resp.Workspaces = append(resp.Workspaces, model.WorkspaceResponse{ID: ws.ID}) + } + c.JSON(http.StatusOK, resp) +} + func (h *workspaceHandler) get(c *gin.Context) { id := c.Param("id") ws, err := h.svc.Get(id) diff --git a/internal/fs/localfs.go b/internal/fs/localfs.go index 131bb1a..63027ce 100644 --- a/internal/fs/localfs.go +++ b/internal/fs/localfs.go @@ -117,10 +117,13 @@ func (l *LocalFS) Mkdir(path string) error { // Remove removes path (file or directory recursively). func (l *LocalFS) Remove(path string) error { - abs, _, err := l.resolve(path) + abs, rel, err := l.resolve(path) if err != nil { return err } + if rel == "." { + return util.New(util.CodeBadRequest, "cannot remove workspace root") + } if err := os.RemoveAll(abs); err != nil { return util.Wrap(util.CodeInternal, "failed to remove path", err) } diff --git a/internal/fs/localfs_test.go b/internal/fs/localfs_test.go index df9911a..f5e49af 100644 --- a/internal/fs/localfs_test.go +++ b/internal/fs/localfs_test.go @@ -135,3 +135,12 @@ func TestLocalFSListSorted(t *testing.T) { t.Errorf("List not sorted: %v", names) } } + +func TestLocalFSRejectsRemoveRoot(t *testing.T) { + fsys := NewLocal(t.TempDir()) + for _, path := range []string{"", "."} { + if err := fsys.Remove(path); err == nil { + t.Fatalf("Remove(%q) expected error", path) + } + } +} diff --git a/internal/model/file.go b/internal/model/file.go index e8d29a5..a2b3e21 100644 --- a/internal/model/file.go +++ b/internal/model/file.go @@ -22,3 +22,13 @@ type ReadFileResponse struct { Path string `json:"path"` Content string `json:"content"` } + +// FileInfoResponse is the API response for file stat. +// Path is always workspace-relative; host absolute paths are never exposed. +type FileInfoResponse struct { + Name string `json:"name"` + Path string `json:"path"` + IsDir bool `json:"isDir"` + Size int64 `json:"size"` + ModTime string `json:"modTime"` +} diff --git a/internal/model/workspace.go b/internal/model/workspace.go index fbc9feb..a3c7d74 100644 --- a/internal/model/workspace.go +++ b/internal/model/workspace.go @@ -9,3 +9,9 @@ type CreateWorkspaceRequest struct { type WorkspaceResponse struct { ID string `json:"id"` } + +// WorkspaceListResponse is the API response for listing workspaces. +// Root is never exposed. +type WorkspaceListResponse struct { + Workspaces []WorkspaceResponse `json:"workspaces"` +} diff --git a/internal/service/file_service.go b/internal/service/file_service.go index de8fd60..5258f77 100644 --- a/internal/service/file_service.go +++ b/internal/service/file_service.go @@ -2,17 +2,23 @@ package service import ( "codespace/internal/fs" + "codespace/internal/util" "codespace/internal/workspace" ) // FileService handles file operations within workspaces. type FileService struct { - workspaces workspace.Manager + workspaces workspace.Manager + maxWriteBytes int64 } -// NewFileService creates a FileService. -func NewFileService(workspaces workspace.Manager) *FileService { - return &FileService{workspaces: workspaces} +// NewFileService creates a FileService with a per-write byte limit. +// If maxWriteBytes is zero or negative, a 1 MiB default is applied. +func NewFileService(workspaces workspace.Manager, maxWriteBytes int64) *FileService { + if maxWriteBytes <= 0 { + maxWriteBytes = 1 << 20 + } + return &FileService{workspaces: workspaces, maxWriteBytes: maxWriteBytes} } // List lists files at the given workspace-relative path. @@ -34,7 +40,11 @@ func (s *FileService) Read(workspaceID, path string) ([]byte, error) { } // Write writes data to a file at the given workspace-relative path. +// Writes exceeding maxWriteBytes are rejected before touching the filesystem. func (s *FileService) Write(workspaceID, path string, data []byte) error { + if int64(len(data)) > s.maxWriteBytes { + return util.New(util.CodeBadRequest, "file exceeds max write size") + } ws, err := s.workspaces.Get(workspaceID) if err != nil { return err diff --git a/internal/service/workspace_service.go b/internal/service/workspace_service.go index 10c3f35..810b5ad 100644 --- a/internal/service/workspace_service.go +++ b/internal/service/workspace_service.go @@ -38,6 +38,11 @@ func (s *WorkspaceService) Get(id string) (*workspace.Workspace, error) { return s.workspaces.Get(id) } +// List returns all workspaces sorted by ID. +func (s *WorkspaceService) List() ([]workspace.Workspace, error) { + return s.workspaces.List() +} + // Delete stops any running process for the workspace, then removes it. func (s *WorkspaceService) Delete(id string) error { status := s.processes.Status(id) diff --git a/internal/workspace/manager.go b/internal/workspace/manager.go index ad34a4a..f91785f 100644 --- a/internal/workspace/manager.go +++ b/internal/workspace/manager.go @@ -3,6 +3,7 @@ package workspace import ( "os" "path/filepath" + "sort" "codespace/internal/util" ) @@ -12,6 +13,7 @@ type Manager interface { Create(id string) (*Workspace, error) Get(id string) (*Workspace, error) Delete(id string) error + List() ([]Workspace, error) } // LocalManager implements Manager using the local filesystem. @@ -71,3 +73,29 @@ func (m *LocalManager) Delete(id string) error { } return nil } + +// List returns all workspaces under the manager root, sorted by ID. +// Only directories whose names pass ValidID are included. +func (m *LocalManager) List() ([]Workspace, error) { + if err := os.MkdirAll(m.root, 0o755); err != nil { + return nil, util.Wrap(util.CodeInternal, "failed to ensure workspace root", err) + } + entries, err := os.ReadDir(m.root) + if err != nil { + return nil, util.Wrap(util.CodeInternal, "failed to read workspace root", err) + } + var result []Workspace + for _, e := range entries { + if !e.IsDir() { + continue + } + if !ValidID(e.Name()) { + continue + } + result = append(result, Workspace{ID: e.Name(), Root: RootFor(m.root, e.Name())}) + } + sort.Slice(result, func(i, j int) bool { + return result[i].ID < result[j].ID + }) + return result, nil +} diff --git a/internal/workspace/manager_test.go b/internal/workspace/manager_test.go new file mode 100644 index 0000000..48765bd --- /dev/null +++ b/internal/workspace/manager_test.go @@ -0,0 +1,51 @@ +package workspace + +import ( + "os" + "path/filepath" + "testing" +) + +func TestLocalManagerListReturnsSortedWorkspaces(t *testing.T) { + mgr := NewLocalManager(t.TempDir()) + if _, err := mgr.Create("beta"); err != nil { + t.Fatal(err) + } + if _, err := mgr.Create("alpha"); err != nil { + t.Fatal(err) + } + + got, err := mgr.List() + if err != nil { + t.Fatal(err) + } + if len(got) != 2 { + t.Fatalf("len = %d, want 2", len(got)) + } + if got[0].ID != "alpha" || got[1].ID != "beta" { + t.Fatalf("ids = %q, %q; want alpha, beta", got[0].ID, got[1].ID) + } + if got[0].Root == "" || got[1].Root == "" { + t.Fatal("manager list should keep internal roots for service use") + } +} + +func TestLocalManagerListSkipsUnsafeDirectoryNames(t *testing.T) { + root := t.TempDir() + if err := os.Mkdir(filepath.Join(root, "valid"), 0755); err != nil { + t.Fatal(err) + } + // Contains space, which is invalid per ValidID regex + if err := os.Mkdir(filepath.Join(root, "bad name"), 0755); err != nil { + t.Fatal(err) + } + + mgr := NewLocalManager(root) + got, err := mgr.List() + if err != nil { + t.Fatal(err) + } + if len(got) != 1 || got[0].ID != "valid" { + t.Fatalf("got %#v, want only valid", got) + } +} diff --git a/pkg/config/config.go b/pkg/config/config.go index e6e6bed..838b5f2 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -14,6 +14,7 @@ type Config struct { Server ServerConfig `yaml:"server"` Workspace WorkspaceConfig `yaml:"workspace"` Process ProcessConfig `yaml:"process"` + File FileConfig `yaml:"file"` Log LogConfig `yaml:"log"` } @@ -36,6 +37,11 @@ type ProcessConfig struct { OpenCodeCommand string `yaml:"opencodeCommand"` } +// FileConfig holds file operation limits. +type FileConfig struct { + MaxWriteBytes int64 `yaml:"maxWriteBytes"` +} + // LogConfig holds structured logging settings. type LogConfig struct { Level string `yaml:"level"` @@ -48,6 +54,7 @@ type rawConfig struct { Server rawServerConfig `yaml:"server"` Workspace WorkspaceConfig `yaml:"workspace"` Process ProcessConfig `yaml:"process"` + File FileConfig `yaml:"file"` Log LogConfig `yaml:"log"` } @@ -71,6 +78,7 @@ func Default() Config { }, Workspace: WorkspaceConfig{Root: "./workspaces"}, Process: ProcessConfig{OpenCodeCommand: "opencode"}, + File: FileConfig{MaxWriteBytes: 1 << 20}, Log: LogConfig{Level: "info", Format: "json"}, } } @@ -121,6 +129,9 @@ func applyRaw(cfg *Config, raw rawConfig) error { if raw.Process.OpenCodeCommand != "" { cfg.Process.OpenCodeCommand = raw.Process.OpenCodeCommand } + if raw.File.MaxWriteBytes != 0 { + cfg.File.MaxWriteBytes = raw.File.MaxWriteBytes + } if raw.Log.Level != "" { cfg.Log.Level = raw.Log.Level } @@ -174,6 +185,13 @@ func applyEnv(cfg *Config) error { if v := os.Getenv("CODESPACE_OPENCODE_COMMAND"); v != "" { cfg.Process.OpenCodeCommand = v } + if v := os.Getenv("CODESPACE_FILE_MAX_WRITE_BYTES"); v != "" { + n, err := strconv.ParseInt(v, 10, 64) + if err != nil { + return fmt.Errorf("invalid integer for CODESPACE_FILE_MAX_WRITE_BYTES: %w", err) + } + cfg.File.MaxWriteBytes = n + } if v := os.Getenv("CODESPACE_LOG_LEVEL"); v != "" { cfg.Log.Level = v } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 5a99441..31bdae8 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -143,3 +143,39 @@ func TestLoadAppliesLogEnvironmentOverrides(t *testing.T) { t.Fatalf("Log.Format = %q, want json", cfg.Log.Format) } } + +func TestDefaultIncludesFileConfig(t *testing.T) { + cfg := Default() + if cfg.File.MaxWriteBytes != 1<<20 { + t.Fatalf("File.MaxWriteBytes = %d, want 1048576", cfg.File.MaxWriteBytes) + } +} + +func TestLoadParsesFileConfigFromYAML(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + data := []byte(`file: + maxWriteBytes: 128 +`) + if err := os.WriteFile(path, data, 0o644); err != nil { + t.Fatal(err) + } + cfg, err := Load(path) + if err != nil { + t.Fatal(err) + } + if cfg.File.MaxWriteBytes != 128 { + t.Fatalf("got %d", cfg.File.MaxWriteBytes) + } +} + +func TestLoadAppliesFileEnvironmentOverride(t *testing.T) { + t.Setenv("CODESPACE_FILE_MAX_WRITE_BYTES", "256") + cfg, err := Load(filepath.Join(t.TempDir(), "missing.yaml")) + if err != nil { + t.Fatal(err) + } + if cfg.File.MaxWriteBytes != 256 { + t.Fatalf("got %d", cfg.File.MaxWriteBytes) + } +}