feat: harden workspace file APIs
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
tao.chen
co-authored by
Claude Fable 5
Claude Fable 5
parent
c1b8982e3b
commit
04b309d3fc
@@ -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.
|
||||
Reference in New Issue
Block a user