From c78f6d1b40b9a86b56d06bcdd6a61222e4b23947 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 7 Jun 2026 17:52:09 +0200 Subject: [PATCH 1/4] fix(httpapi): enforce path-scoped fs grants --- internal/httpapi/auth.go | 98 ++++++++++++++++++---------- internal/httpapi/auth_test.go | 118 ++++++++++++++++++++++++++++++++++ 2 files changed, 182 insertions(+), 34 deletions(-) diff --git a/internal/httpapi/auth.go b/internal/httpapi/auth.go index e36c374f..f09ce502 100644 --- a/internal/httpapi/auth.go +++ b/internal/httpapi/auth.go @@ -437,10 +437,6 @@ func scopeMatches(granted map[string]struct{}, required string) bool { } func scopeMatchesPath(granted map[string]struct{}, required string, filePath string) bool { - if _, ok := granted[required]; ok { - return true - } - filePath = strings.TrimSpace(filePath) parts := strings.SplitN(required, ":", 2) if len(parts) != 2 { @@ -448,47 +444,81 @@ func scopeMatchesPath(granted map[string]struct{}, required string, filePath str } resource, action := parts[0], parts[1] + hasNarrowPathGrant := false for scope := range granted { - segments := strings.SplitN(scope, ":", 4) - if len(segments) < 3 { - continue - } - - plane, res, act := segments[0], segments[1], segments[2] - scopePath := "*" - if len(segments) == 4 { - scopePath = segments[3] - } - - if plane != "relayfile" && plane != "*" { + scopePath, ok := pathScopeForRequired(scope, resource, action) + if !ok { continue } - if res != resource && res != "*" { - continue + if scopePath == "*" || filePath == "" { + return true } - if act != action && act != "*" { - if act != "manage" || (action != "read" && action != "write") { - continue - } + hasNarrowPathGrant = true + if scopePathMatches(scopePath, filePath) { + return true } + } - if scopePath == "*" { - return true + if _, ok := granted[required]; ok { + return !hasNarrowPathGrant + } + return false +} + +func pathScopeForRequired(scope, resource, action string) (string, bool) { + segments := strings.SplitN(scope, ":", 4) + if len(segments) < 3 { + return "", false + } + + switch segments[0] { + case "relayfile", "*": + res, act := segments[1], segments[2] + if res != resource && res != "*" { + return "", false } - if filePath == "" { - return true + if !scopeActionMatches(act, action) { + return "", false } - scopeDir := strings.TrimSuffix(scopePath, "/*") - scopeDir = strings.TrimSuffix(scopeDir, "*") - if scopePath == filePath { - return true + if len(segments) == 4 { + return strings.TrimSpace(segments[3]), true } - if strings.HasSuffix(scopePath, "/*") && strings.HasPrefix(filePath, scopeDir+"/") { - return true + return "*", true + case "workspace": + act := segments[2] + if resource != "fs" || !scopeActionMatches(act, action) { + return "", false } - if strings.HasSuffix(scopePath, "*") && strings.HasPrefix(filePath, scopeDir) { - return true + if len(segments) == 4 { + return strings.TrimSpace(segments[3]), true } + return "*", true + default: + return "", false + } +} + +func scopeActionMatches(granted, required string) bool { + if granted == required || granted == "*" { + return true + } + return granted == "manage" && (required == "read" || required == "write") +} + +func scopePathMatches(scopePath, filePath string) bool { + if scopePath == filePath { + return true + } + if strings.HasSuffix(scopePath, "/**") { + scopeDir := strings.TrimSuffix(scopePath, "/**") + return filePath == scopeDir || strings.HasPrefix(filePath, scopeDir+"/") + } + if strings.HasSuffix(scopePath, "/*") { + scopeDir := strings.TrimSuffix(scopePath, "/*") + return strings.HasPrefix(filePath, scopeDir+"/") + } + if strings.HasSuffix(scopePath, "*") { + return strings.HasPrefix(filePath, strings.TrimSuffix(scopePath, "*")) } return false } diff --git a/internal/httpapi/auth_test.go b/internal/httpapi/auth_test.go index 262ca06b..afba88ea 100644 --- a/internal/httpapi/auth_test.go +++ b/internal/httpapi/auth_test.go @@ -114,6 +114,40 @@ func TestScopeMatchesPath(t *testing.T) { path: "/docs/readme.md", want: true, }, + { + name: "bare read with workspace path grant allows inside subtree", + required: "fs:read", + granted: map[string]struct{}{ + "fs:read": {}, + "workspace:mount-sponsor:read:/slack/messages/**": {}, + }, + path: "/slack/messages/thread-1.json", + want: true, + }, + { + name: "bare read with workspace path grant denies outside subtree", + required: "fs:read", + granted: map[string]struct{}{ + "fs:read": {}, + "workspace:mount-sponsor:read:/slack/messages/**": {}, + }, + path: "/slack/users/user-1.json", + want: false, + }, + { + name: "pure bare read remains full access", + required: "fs:read", + granted: map[string]struct{}{"fs:read": {}}, + path: "/slack/users/user-1.json", + want: true, + }, + { + name: "workspace sponsor segment is not treated as resource", + required: "fs:read", + granted: map[string]struct{}{"workspace:pear-integrations-slack-channels-c123-messages:read:/slack/channels/c123/messages/**": {}}, + path: "/slack/channels/c123/messages/1710000000.json", + want: true, + }, { name: "wrong plane", required: "fs:read", @@ -177,6 +211,90 @@ func TestScopeMatchesPath(t *testing.T) { }) } +func TestAuthorizeBearerEnforcesPathScopedMountGrants(t *testing.T) { + t.Parallel() + + now := time.Now().UTC() + privateKey := mustRSATestKey(t) + + jwksServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _ = json.NewEncoder(w).Encode(jwksDocument{ + Keys: []jwkKey{mustRSATestJWK("kid-1", &privateKey.PublicKey)}, + }) + })) + defer jwksServer.Close() + + verifier := newBearerVerifier(ServerConfig{ + JWKSURL: jwksServer.URL, + JWKSFetchTimeout: time.Second, + }) + + tests := []struct { + name string + scopes []string + path string + wantStatus int + }{ + { + name: "bare fs read plus workspace path grant allows inside subtree", + scopes: []string{"fs:read", "workspace:mount-sponsor:read:/slack/messages/**"}, + path: "/slack/messages/thread-1.json", + wantStatus: 0, + }, + { + name: "bare fs read plus workspace path grant denies outside subtree", + scopes: []string{"fs:read", "workspace:mount-sponsor:read:/slack/messages/**"}, + path: "/slack/users/user-1.json", + wantStatus: http.StatusForbidden, + }, + { + name: "workspace path grant without bare read allows inside subtree", + scopes: []string{"workspace:mount-sponsor:read:/slack/messages/**"}, + path: "/slack/messages/thread-1.json", + wantStatus: 0, + }, + { + name: "workspace path grant without bare read denies outside subtree", + scopes: []string{"workspace:mount-sponsor:read:/slack/messages/**"}, + path: "/slack/users/user-1.json", + wantStatus: http.StatusForbidden, + }, + { + name: "pure bare fs read remains full access", + scopes: []string{"fs:read"}, + path: "/slack/users/user-1.json", + wantStatus: 0, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + token := mustTestRS256JWT(t, privateKey, "kid-1", map[string]any{ + "wks": "ws-mount", + "sub": "MountSync", + "scopes": tt.scopes, + "exp": now.Add(time.Hour).Unix(), + "aud": "relayfile", + }) + + _, authErr := authorizeBearer("Bearer "+token, verifier, "ws-mount", "fs:read", tt.path, now) + if tt.wantStatus == 0 { + if authErr != nil { + t.Fatalf("authorizeBearer returned auth error: %+v", authErr) + } + return + } + if authErr == nil { + t.Fatalf("expected auth error status %d, got nil", tt.wantStatus) + } + if authErr.status != tt.wantStatus { + t.Fatalf("expected auth status %d, got %d (%s)", tt.wantStatus, authErr.status, authErr.message) + } + }) + } +} + func TestParseBearerRS256HappyPath(t *testing.T) { t.Parallel() From 9caab2e2300999dfee2e7d45dd6523963db8a2bf Mon Sep 17 00:00:00 2001 From: "agent-relay-code[bot]" Date: Sun, 7 Jun 2026 15:57:14 +0000 Subject: [PATCH 2/4] chore: apply pr-reviewer fixes for #255 --- .../active/traj_y5jru5dh9ku6/trajectory.json | 43 ++++- .trajectories/index.json | 181 ------------------ 2 files changed, 41 insertions(+), 183 deletions(-) delete mode 100644 .trajectories/index.json diff --git a/.trajectories/active/traj_y5jru5dh9ku6/trajectory.json b/.trajectories/active/traj_y5jru5dh9ku6/trajectory.json index 755ae350..d59e61dd 100644 --- a/.trajectories/active/traj_y5jru5dh9ku6/trajectory.json +++ b/.trajectories/active/traj_y5jru5dh9ku6/trajectory.json @@ -6,8 +6,47 @@ }, "status": "active", "startedAt": "2026-06-06T00:15:50.055Z", - "agents": [], - "chapters": [], + "agents": [ + { + "name": "default", + "role": "lead", + "joinedAt": "2026-06-07T15:55:00.093Z" + } + ], + "chapters": [ + { + "id": "chap_krw4gjdmzw08", + "title": "Work", + "agentName": "default", + "startedAt": "2026-06-07T15:55:00.093Z", + "events": [ + { + "ts": 1780847700094, + "type": "decision", + "content": "Validated PR bot comments before editing: Validated PR bot comments before editing", + "raw": { + "question": "Validated PR bot comments before editing", + "chosen": "Validated PR bot comments before editing", + "alternatives": [], + "reasoning": "Fetched PR #255 discussion via GitHub connector; available bot comments had no actionable findings against current checkout." + }, + "significance": "high" + }, + { + "ts": 1780847812604, + "type": "reflection", + "content": "PR #255 review found no actionable bot findings; focused auth package tests pass after installing Go.", + "raw": { + "confidence": 0.82 + }, + "significance": "high", + "tags": [ + "confidence:0.82" + ] + } + ] + } + ], "commits": [], "filesChanged": [], "projectId": "/home/daytona/workspace", diff --git a/.trajectories/index.json b/.trajectories/index.json deleted file mode 100644 index 92d91136..00000000 --- a/.trajectories/index.json +++ /dev/null @@ -1,181 +0,0 @@ -{ - "version": 1, - "lastUpdated": "2026-06-06T00:23:16.149Z", - "trajectories": { - "traj_7x9nltybo08h": { - "title": "Update PR 65 usage instructions", - "status": "completed", - "startedAt": "2026-04-30T21:07:25.621Z", - "completedAt": "2026-04-30T21:08:20.135Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-04/traj_7x9nltybo08h.json" - }, - "traj_82lywlk9dcnc": { - "title": "Write SDK setup client workflow from spec", - "status": "completed", - "startedAt": "2026-04-30T16:43:24.651Z", - "completedAt": "2026-04-30T16:51:07.147Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-04/traj_82lywlk9dcnc.json" - }, - "traj_cdist8i8vdmd": { - "title": "Address PR 65 feedback", - "status": "completed", - "startedAt": "2026-04-30T20:10:45.916Z", - "completedAt": "2026-04-30T20:11:29.126Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-04/traj_cdist8i8vdmd.json" - }, - "traj_dmoc4slub7ox": { - "title": "Fix SDK setup workflow evidence path", - "status": "completed", - "startedAt": "2026-04-30T17:33:11.390Z", - "completedAt": "2026-04-30T17:34:29.265Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-04/traj_dmoc4slub7ox.json" - }, - "traj_em3hvzpg1xmx": { - "title": "062-sdk-setup-client-workflow", - "status": "completed", - "startedAt": "2026-04-30T17:43:56.110Z", - "completedAt": "2026-04-30T17:43:59.041Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-04/traj_em3hvzpg1xmx.json" - }, - "traj_i1f02867dkxn": { - "title": "Fix SDK setup workflow verify gate", - "status": "completed", - "startedAt": "2026-04-30T17:07:38.811Z", - "completedAt": "2026-04-30T17:10:18.837Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-04/traj_i1f02867dkxn.json" - }, - "traj_iuzm83ogm43k": { - "title": "Replace chokidar with @parcel/watcher in local-mount", - "status": "completed", - "startedAt": "2026-04-20T20:35:15.759Z", - "completedAt": "2026-04-20T20:58:15.412Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-04/traj_iuzm83ogm43k.json" - }, - "traj_mfyus7zfgxt2": { - "title": "062-sdk-setup-client-workflow", - "status": "completed", - "startedAt": "2026-04-30T16:53:07.629Z", - "completedAt": "2026-04-30T17:05:01.326Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-04/traj_mfyus7zfgxt2.json" - }, - "traj_nixaonkglri1": { - "title": "Migrate relayfile e2e and conformance scripts to RS256 local JWKS", - "status": "completed", - "startedAt": "2026-04-24T09:06:31.046Z", - "completedAt": "2026-04-24T09:10:42.425Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-04/traj_nixaonkglri1.json" - }, - "traj_qi3qmy5oveab": { - "title": "Resolve SDK setup review gate", - "status": "completed", - "startedAt": "2026-04-30T17:41:15.457Z", - "completedAt": "2026-04-30T17:43:16.297Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-04/traj_qi3qmy5oveab.json" - }, - "traj_ubq95azheqpt": { - "title": "062-sdk-setup-client-workflow", - "status": "completed", - "startedAt": "2026-04-30T17:12:22.684Z", - "completedAt": "2026-04-30T17:23:57.490Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-04/traj_ubq95azheqpt.json" - }, - "traj_wez7rl7pkfpn": { - "title": "Review PR 65 implementation against spec", - "status": "completed", - "startedAt": "2026-04-30T20:31:59.188Z", - "completedAt": "2026-04-30T20:31:59.361Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-04/traj_wez7rl7pkfpn.json" - }, - "traj_6fjv0fnvrc5e": { - "title": "Relayfile follow-up PRs: cloud conventions, cloud sdk/core bump, adapters release pipeline investigation", - "status": "completed", - "startedAt": "2026-05-09T13:35:32.701Z", - "completedAt": "2026-05-09T13:45:18.302Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-05/traj_6fjv0fnvrc5e.json" - }, - "traj_6lyjg41p6a28": { - "title": "Address PR comments on cloud#504 Linear conventions", - "status": "completed", - "startedAt": "2026-05-09T13:55:09.128Z", - "completedAt": "2026-05-09T13:57:04.293Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-05/traj_6lyjg41p6a28.json" - }, - "traj_9khc36ax639i": { - "title": "Add relayfile eval harness", - "status": "completed", - "startedAt": "2026-05-08T23:08:09.607Z", - "completedAt": "2026-05-08T23:18:24.282Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-05/traj_9khc36ax639i.json" - }, - "traj_a6rfc30zag40": { - "title": "Draft agent workspace golden path spec", - "status": "completed", - "startedAt": "2026-05-01T15:09:58.013Z", - "completedAt": "2026-05-01T15:12:08.390Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-05/traj_a6rfc30zag40.json" - }, - "traj_ailh4waboewf": { - "title": "Design relayfile low-friction cloud login and integration mount flow", - "status": "completed", - "startedAt": "2026-05-01T23:39:39.687Z", - "completedAt": "2026-05-01T23:45:39.611Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-05/traj_ailh4waboewf.json" - }, - "traj_d3drzvodqpn7": { - "title": "Address PR 114 comments", - "status": "completed", - "startedAt": "2026-05-09T08:39:35.473Z", - "completedAt": "2026-05-09T08:42:45.202Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-05/traj_d3drzvodqpn7.json" - }, - "traj_hyqnsfininh5": { - "title": "Write agent workspace implementation workflow", - "status": "completed", - "startedAt": "2026-05-01T15:22:05.684Z", - "completedAt": "2026-05-01T15:27:12.578Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-05/traj_hyqnsfininh5.json" - }, - "traj_nxbsptr6c5q0": { - "title": "Investigate local lag and open status diagnostics PR", - "status": "completed", - "startedAt": "2026-05-14T09:46:57.122Z", - "completedAt": "2026-05-14T09:50:46.065Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-05/traj_nxbsptr6c5q0.json" - }, - "traj_qrt7hh3ht8nk": { - "title": "Investigate confusing relayfile lagging status", - "status": "completed", - "startedAt": "2026-05-14T09:38:05.074Z", - "completedAt": "2026-05-14T09:42:53.566Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-05/traj_qrt7hh3ht8nk.json" - }, - "traj_v1un6n66y38i": { - "title": "Async createMount — issue #104", - "status": "completed", - "startedAt": "2026-05-08T17:27:24.218Z", - "completedAt": "2026-05-08T17:31:22.965Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-05/traj_v1un6n66y38i.json" - }, - "traj_xf18gkmtr3ib": { - "title": "Address PR comments on relayfile-adapters#59", - "status": "completed", - "startedAt": "2026-05-09T13:50:45.476Z", - "completedAt": "2026-05-09T13:54:43.281Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-05/traj_xf18gkmtr3ib.json" - }, - "traj_z2klijcrwqed": { - "title": "Design simple agent workspace connect flow", - "status": "completed", - "startedAt": "2026-05-01T14:58:15.412Z", - "completedAt": "2026-05-01T15:06:23.351Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-05/traj_z2klijcrwqed.json" - }, - "traj_cf89ajbo2ast": { - "title": "Review and fix PR #243", - "status": "completed", - "startedAt": "2026-06-06T00:23:06.923Z", - "completedAt": "2026-06-06T00:23:16.097Z", - "path": "/home/daytona/workspace/.trajectories/completed/2026-06/traj_cf89ajbo2ast.json" - } - } -} \ No newline at end of file From b96d411c1c591bfb196b81e5f3b133b6f1138068 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 7 Jun 2026 17:52:09 +0200 Subject: [PATCH 3/4] fix(httpapi): enforce path-scoped fs grants --- internal/httpapi/server.go | 2 + internal/httpapi/server_test.go | 176 ++++++++++++++++++++++++++++++++ 2 files changed, 178 insertions(+) diff --git a/internal/httpapi/server.go b/internal/httpapi/server.go index 1742e81c..5cb84e54 100644 --- a/internal/httpapi/server.go +++ b/internal/httpapi/server.go @@ -243,6 +243,8 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { switch route { case "read_file", "write_file", "delete_file": scopePath = strings.TrimSpace(r.URL.Query().Get("path")) + case "export", "query_files": + scopePath = normalizeRoutePath(r.URL.Query().Get("path")) } } claims, authErr := authorizeBearer(r.Header.Get("Authorization"), s.bearerVerifier, workspaceID, requiredScope, scopePath, time.Now().UTC()) diff --git a/internal/httpapi/server_test.go b/internal/httpapi/server_test.go index a77e3f61..dbfaee37 100644 --- a/internal/httpapi/server_test.go +++ b/internal/httpapi/server_test.go @@ -1277,6 +1277,94 @@ func TestExportJSONPathFilter(t *testing.T) { } } +func TestExportEnforcesPathScopedMountGrant(t *testing.T) { + store := relayfile.NewStoreWithOptions(relayfile.StoreOptions{DisableWorkers: true}) + t.Cleanup(store.Close) + + workspaceID := "ws_export_scope" + if written, _, errs := store.BulkWrite(workspaceID, []relayfile.BulkWriteFile{ + {Path: "/allowed/A.md", ContentType: "text/markdown", Content: "# A"}, + {Path: "/secret/B.md", ContentType: "text/markdown", Content: "# B"}, + }); written != 2 || len(errs) != 0 { + t.Fatalf("seed bulk write failed: written=%d errs=%+v", written, errs) + } + + server := NewServer(store) + scopedToken := mustTestJWT(t, "dev-secret", workspaceID, "MountSync", []string{ + "fs:read", + "workspace:mount-sponsor:read:/allowed/**", + }, time.Now().Add(time.Hour)) + fullToken := mustTestJWT(t, "dev-secret", workspaceID, "Worker1", []string{"fs:read"}, time.Now().Add(time.Hour)) + + tests := []struct { + name string + token string + path string + wantCode int + wantFiles []string + }{ + { + name: "path-scoped mount token exports inside subtree", + token: scopedToken, + path: "/v1/workspaces/" + workspaceID + "/fs/export?format=json&path=/allowed", + wantCode: http.StatusOK, + wantFiles: []string{"/allowed/A.md"}, + }, + { + name: "path-scoped mount token cannot export outside subtree", + token: scopedToken, + path: "/v1/workspaces/" + workspaceID + "/fs/export?format=json&path=/secret", + wantCode: http.StatusForbidden, + }, + { + name: "path-scoped mount token cannot export whole workspace by omitting path", + token: scopedToken, + path: "/v1/workspaces/" + workspaceID + "/fs/export?format=json", + wantCode: http.StatusForbidden, + }, + { + name: "pure bare fs read token can still export whole workspace", + token: fullToken, + path: "/v1/workspaces/" + workspaceID + "/fs/export?format=json", + wantCode: http.StatusOK, + wantFiles: []string{"/allowed/A.md", "/secret/B.md"}, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + resp := doRequest(t, server, request{ + method: http.MethodGet, + path: tt.path, + headers: map[string]string{ + "Authorization": "Bearer " + tt.token, + "X-Correlation-Id": "corr_export_scope", + }, + }) + if resp.Code != tt.wantCode { + t.Fatalf("expected %d, got %d (%s)", tt.wantCode, resp.Code, resp.Body.String()) + } + if tt.wantCode != http.StatusOK { + return + } + + var files []relayfile.File + if err := json.NewDecoder(resp.Body).Decode(&files); err != nil { + t.Fatalf("decode export response: %v", err) + } + if len(files) != len(tt.wantFiles) { + t.Fatalf("expected %d exported files, got %d: %+v", len(tt.wantFiles), len(files), files) + } + for i, wantPath := range tt.wantFiles { + if files[i].Path != wantPath { + t.Fatalf("expected exported file %d path %q, got %q", i, wantPath, files[i].Path) + } + } + }) + } +} + func TestExportTar(t *testing.T) { store := relayfile.NewStoreWithOptions(relayfile.StoreOptions{DisableWorkers: true}) t.Cleanup(store.Close) @@ -1718,6 +1806,94 @@ func TestQueryFilesEndpoint(t *testing.T) { } } +func TestQueryFilesEnforcesPathScopedMountGrant(t *testing.T) { + store := relayfile.NewStoreWithOptions(relayfile.StoreOptions{DisableWorkers: true}) + t.Cleanup(store.Close) + + workspaceID := "ws_query_scope" + if written, _, errs := store.BulkWrite(workspaceID, []relayfile.BulkWriteFile{ + {Path: "/allowed/A.md", ContentType: "text/markdown", Content: "# A"}, + {Path: "/secret/B.md", ContentType: "text/markdown", Content: "# B"}, + }); written != 2 || len(errs) != 0 { + t.Fatalf("seed bulk write failed: written=%d errs=%+v", written, errs) + } + + server := NewServer(store) + scopedToken := mustTestJWT(t, "dev-secret", workspaceID, "MountSync", []string{ + "fs:read", + "workspace:mount-sponsor:read:/allowed/**", + }, time.Now().Add(time.Hour)) + fullToken := mustTestJWT(t, "dev-secret", workspaceID, "Worker1", []string{"fs:read"}, time.Now().Add(time.Hour)) + + tests := []struct { + name string + token string + path string + wantCode int + wantItems []string + }{ + { + name: "path-scoped mount token queries inside subtree", + token: scopedToken, + path: "/v1/workspaces/" + workspaceID + "/fs/query?path=/allowed", + wantCode: http.StatusOK, + wantItems: []string{"/allowed/A.md"}, + }, + { + name: "path-scoped mount token cannot query outside subtree", + token: scopedToken, + path: "/v1/workspaces/" + workspaceID + "/fs/query?path=/secret", + wantCode: http.StatusForbidden, + }, + { + name: "path-scoped mount token cannot query whole workspace by omitting path", + token: scopedToken, + path: "/v1/workspaces/" + workspaceID + "/fs/query", + wantCode: http.StatusForbidden, + }, + { + name: "pure bare fs read token can still query whole workspace", + token: fullToken, + path: "/v1/workspaces/" + workspaceID + "/fs/query", + wantCode: http.StatusOK, + wantItems: []string{"/allowed/A.md", "/secret/B.md"}, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + resp := doRequest(t, server, request{ + method: http.MethodGet, + path: tt.path, + headers: map[string]string{ + "Authorization": "Bearer " + tt.token, + "X-Correlation-Id": "corr_query_scope", + }, + }) + if resp.Code != tt.wantCode { + t.Fatalf("expected %d, got %d (%s)", tt.wantCode, resp.Code, resp.Body.String()) + } + if tt.wantCode != http.StatusOK { + return + } + + var payload relayfile.FileQueryResponse + if err := json.NewDecoder(resp.Body).Decode(&payload); err != nil { + t.Fatalf("decode query response: %v", err) + } + if len(payload.Items) != len(tt.wantItems) { + t.Fatalf("expected %d query items, got %d: %+v", len(tt.wantItems), len(payload.Items), payload.Items) + } + for i, wantPath := range tt.wantItems { + if payload.Items[i].Path != wantPath { + t.Fatalf("expected query item %d path %q, got %q", i, wantPath, payload.Items[i].Path) + } + } + }) + } +} + func TestFilePermissionPolicyScopeEnforced(t *testing.T) { server := NewServer(relayfile.NewStore()) ownerToken := mustTestJWT(t, "dev-secret", "ws_perm_scope", "Owner", []string{"fs:read", "fs:write", "finance"}, time.Now().Add(time.Hour)) From 49f2ddb7a7a808a6cab7b2f0958cd202f2aee56f Mon Sep 17 00:00:00 2001 From: "agent-relay-code[bot]" Date: Sun, 7 Jun 2026 16:09:41 +0000 Subject: [PATCH 4/4] chore: apply pr-reviewer fixes for #255 --- .../active/traj_y5jru5dh9ku6/trajectory.json | 48 ++++++ internal/httpapi/auth.go | 5 +- internal/httpapi/auth_test.go | 32 +++- internal/httpapi/server.go | 4 + internal/httpapi/server_test.go | 138 ++++++++++++++++++ internal/httpapi/websocket.go | 16 +- 6 files changed, 239 insertions(+), 4 deletions(-) diff --git a/.trajectories/active/traj_y5jru5dh9ku6/trajectory.json b/.trajectories/active/traj_y5jru5dh9ku6/trajectory.json index d59e61dd..cab9d1e6 100644 --- a/.trajectories/active/traj_y5jru5dh9ku6/trajectory.json +++ b/.trajectories/active/traj_y5jru5dh9ku6/trajectory.json @@ -43,6 +43,54 @@ "tags": [ "confidence:0.82" ] + }, + { + "ts": 1780848138859, + "type": "decision", + "content": "Fixed narrowed grant empty-path match: Fixed narrowed grant empty-path match", + "raw": { + "question": "Fixed narrowed grant empty-path match", + "chosen": "Fixed narrowed grant empty-path match", + "alternatives": [], + "reasoning": "Current scopeMatchesPath returned true for any applicable narrowed scope when filePath was empty; only wildcard path scopes should bypass path matching." + }, + "significance": "high" + }, + { + "ts": 1780848308628, + "type": "decision", + "content": "Extended path-scoped fs enforcement to tree/events/websocket: Extended path-scoped fs enforcement to tree/events/websocket", + "raw": { + "question": "Extended path-scoped fs enforcement to tree/events/websocket", + "chosen": "Extended path-scoped fs enforcement to tree/events/websocket", + "alternatives": [], + "reasoning": "Tracing fs:read callers showed tree and websocket already accept path filters but authorized with an empty required path, and events has no path filter; narrowed mount grants should not authorize whole-workspace reads." + }, + "significance": "high" + }, + { + "ts": 1780848332832, + "type": "reflection", + "content": "Validated bot finding and caller trace; first websocket test run exposed a no-path slice guard issue now fixed.", + "raw": { + "confidence": 0.78 + }, + "significance": "high", + "tags": [ + "confidence:0.78" + ] + }, + { + "ts": 1780848567249, + "type": "reflection", + "content": "PR #255 review addressed CodeRabbit's empty-path finding, added fs:write coverage, and closed related path-scoped fs read gaps for tree, events, and websocket subscriptions. Verified with internal/httpapi tests, full Go tests, Go builds, go vet, and contract surface check.", + "raw": { + "confidence": 0.86 + }, + "significance": "high", + "tags": [ + "confidence:0.86" + ] } ] } diff --git a/internal/httpapi/auth.go b/internal/httpapi/auth.go index f09ce502..27e92430 100644 --- a/internal/httpapi/auth.go +++ b/internal/httpapi/auth.go @@ -450,10 +450,13 @@ func scopeMatchesPath(granted map[string]struct{}, required string, filePath str if !ok { continue } - if scopePath == "*" || filePath == "" { + if scopePath == "*" { return true } hasNarrowPathGrant = true + if filePath == "" { + continue + } if scopePathMatches(scopePath, filePath) { return true } diff --git a/internal/httpapi/auth_test.go b/internal/httpapi/auth_test.go index afba88ea..f7399d66 100644 --- a/internal/httpapi/auth_test.go +++ b/internal/httpapi/auth_test.go @@ -134,6 +134,16 @@ func TestScopeMatchesPath(t *testing.T) { path: "/slack/users/user-1.json", want: false, }, + { + name: "bare read with workspace path grant denies empty path", + required: "fs:read", + granted: map[string]struct{}{ + "fs:read": {}, + "workspace:mount-sponsor:read:/slack/messages/**": {}, + }, + path: "", + want: false, + }, { name: "pure bare read remains full access", required: "fs:read", @@ -231,40 +241,60 @@ func TestAuthorizeBearerEnforcesPathScopedMountGrants(t *testing.T) { tests := []struct { name string + required string scopes []string path string wantStatus int }{ { name: "bare fs read plus workspace path grant allows inside subtree", + required: "fs:read", scopes: []string{"fs:read", "workspace:mount-sponsor:read:/slack/messages/**"}, path: "/slack/messages/thread-1.json", wantStatus: 0, }, { name: "bare fs read plus workspace path grant denies outside subtree", + required: "fs:read", scopes: []string{"fs:read", "workspace:mount-sponsor:read:/slack/messages/**"}, path: "/slack/users/user-1.json", wantStatus: http.StatusForbidden, }, { name: "workspace path grant without bare read allows inside subtree", + required: "fs:read", scopes: []string{"workspace:mount-sponsor:read:/slack/messages/**"}, path: "/slack/messages/thread-1.json", wantStatus: 0, }, { name: "workspace path grant without bare read denies outside subtree", + required: "fs:read", scopes: []string{"workspace:mount-sponsor:read:/slack/messages/**"}, path: "/slack/users/user-1.json", wantStatus: http.StatusForbidden, }, { name: "pure bare fs read remains full access", + required: "fs:read", scopes: []string{"fs:read"}, path: "/slack/users/user-1.json", wantStatus: 0, }, + { + name: "bare fs write plus workspace path grant allows inside subtree", + required: "fs:write", + scopes: []string{"fs:write", "workspace:mount-sponsor:write:/slack/messages/**"}, + path: "/slack/messages/thread-1.json", + wantStatus: 0, + }, + { + name: "bare fs write plus workspace path grant denies outside subtree", + required: "fs:write", + scopes: []string{"fs:write", "workspace:mount-sponsor:write:/slack/messages/**"}, + path: "/slack/users/user-1.json", + wantStatus: http.StatusForbidden, + }, } for _, tt := range tests { @@ -278,7 +308,7 @@ func TestAuthorizeBearerEnforcesPathScopedMountGrants(t *testing.T) { "aud": "relayfile", }) - _, authErr := authorizeBearer("Bearer "+token, verifier, "ws-mount", "fs:read", tt.path, now) + _, authErr := authorizeBearer("Bearer "+token, verifier, "ws-mount", tt.required, tt.path, now) if tt.wantStatus == 0 { if authErr != nil { t.Fatalf("authorizeBearer returned auth error: %+v", authErr) diff --git a/internal/httpapi/server.go b/internal/httpapi/server.go index 5cb84e54..ab570f92 100644 --- a/internal/httpapi/server.go +++ b/internal/httpapi/server.go @@ -241,10 +241,14 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { scopePath := "" if requiredScope == "fs:read" || requiredScope == "fs:write" { switch route { + case "tree": + scopePath = normalizeRoutePath(r.URL.Query().Get("path")) case "read_file", "write_file", "delete_file": scopePath = strings.TrimSpace(r.URL.Query().Get("path")) case "export", "query_files": scopePath = normalizeRoutePath(r.URL.Query().Get("path")) + case "events": + scopePath = "/" } } claims, authErr := authorizeBearer(r.Header.Get("Authorization"), s.bearerVerifier, workspaceID, requiredScope, scopePath, time.Now().UTC()) diff --git a/internal/httpapi/server_test.go b/internal/httpapi/server_test.go index dbfaee37..63a6c045 100644 --- a/internal/httpapi/server_test.go +++ b/internal/httpapi/server_test.go @@ -369,6 +369,69 @@ func TestFileEventsWebSocketPathFilterConstrainsServerFanout(t *testing.T) { } } +func TestFileEventsWebSocketEnforcesPathScopedMountGrant(t *testing.T) { + store := relayfile.NewStoreWithOptions(relayfile.StoreOptions{DisableWorkers: true}) + t.Cleanup(store.Close) + + server := httptest.NewServer(NewServer(store)) + defer server.Close() + + token := mustTestJWT(t, "dev-secret", "ws_socket_scope", "MountSync", []string{ + "fs:read", + "workspace:mount-sponsor:read:/allowed/**", + }, time.Now().Add(time.Hour)) + + tests := []struct { + name string + query string + wantCode int + }{ + { + name: "allows subscribed path inside grant", + query: "&from=now&path=" + url.QueryEscape("/allowed/**"), + wantCode: http.StatusSwitchingProtocols, + }, + { + name: "denies subscribed path outside grant", + query: "&from=now&path=" + url.QueryEscape("/secret/**"), + wantCode: http.StatusForbidden, + }, + { + name: "denies whole workspace subscription", + query: "&from=now", + wantCode: http.StatusForbidden, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + wsURL := "ws" + strings.TrimPrefix(server.URL, "http") + "/v1/workspaces/ws_socket_scope/fs/ws?token=" + token + tt.query + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + conn, resp, err := websocket.Dial(ctx, wsURL, nil) + if tt.wantCode == http.StatusSwitchingProtocols { + if err != nil { + t.Fatalf("websocket dial failed: %v", err) + } + defer conn.Close(websocket.StatusNormalClosure, "") + return + } + if err == nil { + defer conn.Close(websocket.StatusNormalClosure, "") + t.Fatalf("expected websocket dial to fail with %d", tt.wantCode) + } + if resp == nil { + t.Fatalf("expected websocket dial response with status %d, got nil response: %v", tt.wantCode, err) + } + if resp.StatusCode != tt.wantCode { + t.Fatalf("expected websocket status %d, got %d", tt.wantCode, resp.StatusCode) + } + }) + } +} + func TestFileEventsWebSocketWritebackMaterializationCarriesAgentWriteOrigin(t *testing.T) { store := relayfile.NewStoreWithOptions(relayfile.StoreOptions{ ProviderWriteAction: func(action relayfile.WritebackAction) error { @@ -1894,6 +1957,81 @@ func TestQueryFilesEnforcesPathScopedMountGrant(t *testing.T) { } } +func TestTreeAndEventsEnforcePathScopedMountGrant(t *testing.T) { + store := relayfile.NewStoreWithOptions(relayfile.StoreOptions{DisableWorkers: true}) + t.Cleanup(store.Close) + + workspaceID := "ws_tree_events_scope" + if written, _, errs := store.BulkWrite(workspaceID, []relayfile.BulkWriteFile{ + {Path: "/allowed/A.md", ContentType: "text/markdown", Content: "# A"}, + {Path: "/secret/B.md", ContentType: "text/markdown", Content: "# B"}, + }); written != 2 || len(errs) != 0 { + t.Fatalf("seed bulk write failed: written=%d errs=%+v", written, errs) + } + + server := NewServer(store) + scopedToken := mustTestJWT(t, "dev-secret", workspaceID, "MountSync", []string{ + "fs:read", + "workspace:mount-sponsor:read:/allowed/**", + }, time.Now().Add(time.Hour)) + fullToken := mustTestJWT(t, "dev-secret", workspaceID, "Worker1", []string{"fs:read"}, time.Now().Add(time.Hour)) + + tests := []struct { + name string + token string + path string + wantCode int + }{ + { + name: "path-scoped mount token can read tree inside subtree", + token: scopedToken, + path: "/v1/workspaces/" + workspaceID + "/fs/tree?path=/allowed", + wantCode: http.StatusOK, + }, + { + name: "path-scoped mount token cannot read tree outside subtree", + token: scopedToken, + path: "/v1/workspaces/" + workspaceID + "/fs/tree?path=/secret", + wantCode: http.StatusForbidden, + }, + { + name: "path-scoped mount token cannot read whole tree by omitting path", + token: scopedToken, + path: "/v1/workspaces/" + workspaceID + "/fs/tree", + wantCode: http.StatusForbidden, + }, + { + name: "path-scoped mount token cannot read whole event stream", + token: scopedToken, + path: "/v1/workspaces/" + workspaceID + "/fs/events?limit=10", + wantCode: http.StatusForbidden, + }, + { + name: "pure bare fs read token can still read whole event stream", + token: fullToken, + path: "/v1/workspaces/" + workspaceID + "/fs/events?limit=10", + wantCode: http.StatusOK, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + resp := doRequest(t, server, request{ + method: http.MethodGet, + path: tt.path, + headers: map[string]string{ + "Authorization": "Bearer " + tt.token, + "X-Correlation-Id": "corr_tree_events_scope", + }, + }) + if resp.Code != tt.wantCode { + t.Fatalf("expected %d, got %d (%s)", tt.wantCode, resp.Code, resp.Body.String()) + } + }) + } +} + func TestFilePermissionPolicyScopeEnforced(t *testing.T) { server := NewServer(relayfile.NewStore()) ownerToken := mustTestJWT(t, "dev-secret", "ws_perm_scope", "Owner", []string{"fs:read", "fs:write", "finance"}, time.Now().Add(time.Hour)) diff --git a/internal/httpapi/websocket.go b/internal/httpapi/websocket.go index 105fd083..427cab02 100644 --- a/internal/httpapi/websocket.go +++ b/internal/httpapi/websocket.go @@ -35,11 +35,24 @@ type websocketSubscriptionOptions struct { } func (s *Server) handleFileEventsWebSocket(w http.ResponseWriter, r *http.Request, workspaceID string) { - claims, authErr := authorizeBearer("Bearer "+strings.TrimSpace(r.URL.Query().Get("token")), s.bearerVerifier, workspaceID, "fs:read", "", time.Now().UTC()) + options := parseWebSocketSubscriptionOptions(r) + scopePath := "/" + if len(options.Paths) > 0 { + scopePath = options.Paths[0] + } + claims, authErr := authorizeBearer("Bearer "+strings.TrimSpace(r.URL.Query().Get("token")), s.bearerVerifier, workspaceID, "fs:read", scopePath, time.Now().UTC()) if authErr != nil { writeError(w, authErr.status, authErr.code, authErr.message, "") return } + if len(options.Paths) > 1 { + for _, path := range options.Paths[1:] { + if !scopeMatchesPath(claims.Scopes, "fs:read", path) { + writeError(w, http.StatusForbidden, "forbidden", "missing required scope: fs:read", "") + return + } + } + } if s.rateLimiter != nil { key := workspaceID + "|" + claims.AgentName if !s.rateLimiter.allow(key, time.Now().UTC()) { @@ -57,7 +70,6 @@ func (s *Server) handleFileEventsWebSocket(w http.ResponseWriter, r *http.Reques defer conn.Close(websocket.StatusNormalClosure, "") ctx := r.Context() - options := parseWebSocketSubscriptionOptions(r) // Subscribe FIRST, then catch up, so no events are missed in between. subscriptionCh := make(chan relayfile.Event, 256)