Changeset 0.37.1 (#213)
Co-authored-by: gobha <jasafpro@gmail.com> Co-committed-by: gobha <jasafpro@gmail.com>
This commit is contained in:
395
CHANGESET.md
Normal file
395
CHANGESET.md
Normal file
@@ -0,0 +1,395 @@
|
||||
# CS-0.37.1 — Permission Audit & Enforcement
|
||||
|
||||
**Date:** 2026-03-20
|
||||
**Scope:** Backend permission enforcement + new self-service permissions endpoint
|
||||
**CI Impact:** Adds new test file with 12 test functions. Existing tests unaffected.
|
||||
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
### Problem
|
||||
- 8 protected routes enforced only `Auth()` (logged-in check) but not
|
||||
`RequirePermission(perm)`. Any authenticated user could call them
|
||||
regardless of group permissions.
|
||||
- No user-facing permissions endpoint — the frontend has zero ability to
|
||||
know what the user is allowed to do.
|
||||
- The Everyone group seeds with `model.use`, `kb.read`, `channel.create`,
|
||||
masking the enforcement gaps in most deployments.
|
||||
- All existing task tests run as admin, which bypasses RequirePermission
|
||||
entirely — permission denial was never tested.
|
||||
|
||||
### Fix
|
||||
1. Add `RequirePermission` middleware to 8 routes in `main.go`
|
||||
2. Mirror the same 8 fixes in the integration test harness
|
||||
3. Add `GET /profile/permissions` endpoint (new handler file)
|
||||
4. Add `GET /profile/permissions` to route registration + test harness
|
||||
5. Add 12 permission enforcement integration tests (deny → grant → allow)
|
||||
|
||||
### New Files (drop-in)
|
||||
- `server/handlers/profile_permissions.go`
|
||||
- `server/handlers/perm_enforcement_test.go`
|
||||
|
||||
### Edited Files (str_replace patches below)
|
||||
- `server/main.go` — 9 route edits
|
||||
- `server/handlers/integration_test.go` — 9 route edits (mirror)
|
||||
- `docs/ICD/profile.md` — add new endpoint documentation
|
||||
|
||||
---
|
||||
|
||||
## Patch Instructions
|
||||
|
||||
Apply each str_replace in order. Each section shows the exact old string
|
||||
and the exact replacement.
|
||||
|
||||
### 1. main.go — Completions (model.use)
|
||||
|
||||
**Old:**
|
||||
```
|
||||
protected.POST("/chat/completions", comp.Complete)
|
||||
```
|
||||
|
||||
**New:**
|
||||
```
|
||||
protected.POST("/chat/completions", middleware.RequirePermission(auth.PermModelUse, stores), comp.Complete)
|
||||
```
|
||||
|
||||
### 2. main.go — Summarize (model.use)
|
||||
|
||||
**Old:**
|
||||
```
|
||||
protected.POST("/channels/:id/summarize", summarize.Summarize)
|
||||
```
|
||||
|
||||
**New:**
|
||||
```
|
||||
protected.POST("/channels/:id/summarize", middleware.RequirePermission(auth.PermModelUse, stores), summarize.Summarize)
|
||||
```
|
||||
|
||||
### 3. main.go — Generate Title (model.use)
|
||||
|
||||
**Old:**
|
||||
```
|
||||
protected.POST("/channels/:id/generate-title", titleH.GenerateTitle)
|
||||
```
|
||||
|
||||
**New:**
|
||||
```
|
||||
protected.POST("/channels/:id/generate-title", middleware.RequirePermission(auth.PermModelUse, stores), titleH.GenerateTitle)
|
||||
```
|
||||
|
||||
### 4. main.go — KB Search (kb.read)
|
||||
|
||||
**Old:**
|
||||
```
|
||||
protected.POST("/knowledge-bases/:id/search", kbH.SearchKB)
|
||||
```
|
||||
|
||||
**New:**
|
||||
```
|
||||
protected.POST("/knowledge-bases/:id/search", middleware.RequirePermission(auth.PermKBRead, stores), kbH.SearchKB)
|
||||
```
|
||||
|
||||
### 5. main.go — KB Update (kb.write)
|
||||
|
||||
**Old:**
|
||||
```
|
||||
protected.PUT("/knowledge-bases/:id", kbH.UpdateKB)
|
||||
```
|
||||
|
||||
**New:**
|
||||
```
|
||||
protected.PUT("/knowledge-bases/:id", middleware.RequirePermission(auth.PermKBWrite, stores), kbH.UpdateKB)
|
||||
```
|
||||
|
||||
### 6. main.go — KB Delete (kb.write)
|
||||
|
||||
**Old:**
|
||||
```
|
||||
protected.DELETE("/knowledge-bases/:id", kbH.DeleteKB)
|
||||
```
|
||||
|
||||
**New:**
|
||||
```
|
||||
protected.DELETE("/knowledge-bases/:id", middleware.RequirePermission(auth.PermKBWrite, stores), kbH.DeleteKB)
|
||||
```
|
||||
|
||||
### 7. main.go — KB Document Delete (kb.write)
|
||||
|
||||
**Old:**
|
||||
```
|
||||
protected.DELETE("/knowledge-bases/:id/documents/:docId", kbH.DeleteDocument)
|
||||
```
|
||||
|
||||
**New:**
|
||||
```
|
||||
protected.DELETE("/knowledge-bases/:id/documents/:docId", middleware.RequirePermission(auth.PermKBWrite, stores), kbH.DeleteDocument)
|
||||
```
|
||||
|
||||
### 8. main.go — KB Rebuild (kb.write)
|
||||
|
||||
**Old:**
|
||||
```
|
||||
protected.POST("/knowledge-bases/:id/rebuild", kbH.RebuildKB)
|
||||
```
|
||||
|
||||
**New:**
|
||||
```
|
||||
protected.POST("/knowledge-bases/:id/rebuild", middleware.RequirePermission(auth.PermKBWrite, stores), kbH.RebuildKB)
|
||||
```
|
||||
|
||||
### 9. main.go — New route: GET /profile/permissions
|
||||
|
||||
Insert AFTER the existing profile routes block. Find:
|
||||
|
||||
```
|
||||
protected.GET("/settings", settings.GetSettings)
|
||||
protected.PUT("/settings", settings.UpdateSettings)
|
||||
```
|
||||
|
||||
Insert immediately after:
|
||||
|
||||
```
|
||||
// Permission bootstrap (v0.37.1) — self-service resolved permissions
|
||||
permH := handlers.NewProfilePermissionsHandler(stores)
|
||||
protected.GET("/profile/permissions", permH.GetMyPermissions)
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 10. integration_test.go — Mirror: Completions (model.use)
|
||||
|
||||
**Old:**
|
||||
```
|
||||
protected.POST("/chat/completions", completions.Complete)
|
||||
```
|
||||
|
||||
**New:**
|
||||
```
|
||||
protected.POST("/chat/completions", middleware.RequirePermission(authpkg.PermModelUse, stores), completions.Complete)
|
||||
```
|
||||
|
||||
### 11. integration_test.go — Mirror: KB Search (kb.read)
|
||||
|
||||
**Old:**
|
||||
```
|
||||
protected.POST("/knowledge-bases/:id/search", kbH.SearchKB)
|
||||
```
|
||||
|
||||
**New:**
|
||||
```
|
||||
protected.POST("/knowledge-bases/:id/search", middleware.RequirePermission(authpkg.PermKBRead, stores), kbH.SearchKB)
|
||||
```
|
||||
|
||||
### 12. integration_test.go — Mirror: KB Update (kb.write)
|
||||
|
||||
**Old:**
|
||||
```
|
||||
protected.PUT("/knowledge-bases/:id", kbH.UpdateKB)
|
||||
```
|
||||
|
||||
**New:**
|
||||
```
|
||||
protected.PUT("/knowledge-bases/:id", middleware.RequirePermission(authpkg.PermKBWrite, stores), kbH.UpdateKB)
|
||||
```
|
||||
|
||||
### 13. integration_test.go — Mirror: KB Delete (kb.write)
|
||||
|
||||
**Old:**
|
||||
```
|
||||
protected.DELETE("/knowledge-bases/:id", kbH.DeleteKB)
|
||||
```
|
||||
|
||||
**New:**
|
||||
```
|
||||
protected.DELETE("/knowledge-bases/:id", middleware.RequirePermission(authpkg.PermKBWrite, stores), kbH.DeleteKB)
|
||||
```
|
||||
|
||||
### 14. integration_test.go — Mirror: KB Doc Delete (kb.write)
|
||||
|
||||
**Old:**
|
||||
```
|
||||
protected.DELETE("/knowledge-bases/:id/documents/:docId", kbH.DeleteDocument)
|
||||
```
|
||||
|
||||
**New:**
|
||||
```
|
||||
protected.DELETE("/knowledge-bases/:id/documents/:docId", middleware.RequirePermission(authpkg.PermKBWrite, stores), kbH.DeleteDocument)
|
||||
```
|
||||
|
||||
### 15. integration_test.go — Mirror: KB Rebuild (kb.write)
|
||||
|
||||
**Old:**
|
||||
```
|
||||
protected.POST("/knowledge-bases/:id/rebuild", kbH.RebuildKB)
|
||||
```
|
||||
|
||||
**New:**
|
||||
```
|
||||
protected.POST("/knowledge-bases/:id/rebuild", middleware.RequirePermission(authpkg.PermKBWrite, stores), kbH.RebuildKB)
|
||||
```
|
||||
|
||||
### 16. integration_test.go — New route: GET /profile/permissions
|
||||
|
||||
Find this block in setupHarness:
|
||||
|
||||
```
|
||||
// Settings / Profile
|
||||
settings := NewSettingsHandler(stores, nil)
|
||||
```
|
||||
|
||||
Insert BEFORE it:
|
||||
|
||||
```
|
||||
// Profile permissions (v0.37.1)
|
||||
permH := NewProfilePermissionsHandler(stores)
|
||||
protected.GET("/profile/permissions", permH.GetMyPermissions)
|
||||
|
||||
```
|
||||
|
||||
### 17. Existing KB Permission Test Update
|
||||
|
||||
The existing `TestIntegration_KB_PermissionEnforcement` test (line ~2808)
|
||||
tests kb.create denial. It still passes because it relies on TruncateAll
|
||||
wiping the Everyone group. No changes needed.
|
||||
|
||||
However, note that this test's comment says:
|
||||
```
|
||||
// (TruncateAll wipes the Everyone group, so no default permissions)
|
||||
```
|
||||
This is the only reason it works. On a live system with an intact Everyone
|
||||
group seeded with `kb.read`, the denial would not trigger for kb.read
|
||||
routes. The new `perm_enforcement_test.go` tests do NOT rely on Everyone
|
||||
group presence/absence — they work from a clean slate.
|
||||
|
||||
---
|
||||
|
||||
## ICD Update
|
||||
|
||||
Add to `docs/ICD/profile.md` before the closing `---`:
|
||||
|
||||
```markdown
|
||||
### Get My Permissions
|
||||
|
||||
\```
|
||||
GET /profile/permissions
|
||||
\```
|
||||
|
||||
**Auth:** Authenticated user
|
||||
|
||||
Returns the current user's resolved permission set, contributing group
|
||||
IDs, team memberships, and UI-relevant policies. Admin users receive all
|
||||
permissions.
|
||||
|
||||
**Response:**
|
||||
|
||||
\```json
|
||||
{
|
||||
"permissions": ["model.use", "kb.read", "channel.create"],
|
||||
"groups": ["group-id-1", "00000000-0000-0000-0000-000000000001"],
|
||||
"teams": [
|
||||
{ "id": "uuid", "name": "Engineering", "my_role": "member" }
|
||||
],
|
||||
"policies": {
|
||||
"allow_user_byok": false,
|
||||
"allow_user_personas": false,
|
||||
"allow_raw_model_access": true,
|
||||
"kb_direct_access": true
|
||||
}
|
||||
}
|
||||
\```
|
||||
|
||||
| Field | Type | Description |
|
||||
|-------|------|-------------|
|
||||
| `permissions` | string[] | Resolved permission set (union of all groups) |
|
||||
| `groups` | string[] | Contributing group IDs (always includes Everyone) |
|
||||
| `teams` | object[] | User's active team memberships with role |
|
||||
| `policies` | object | Boolean policy flags relevant to UI feature gating |
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Audit Summary
|
||||
|
||||
### Routes Now Enforcing RequirePermission
|
||||
|
||||
| Route | Permission | Was | Now |
|
||||
|-------|-----------|-----|-----|
|
||||
| `POST /chat/completions` | `model.use` | Auth() only | RequirePermission |
|
||||
| `POST /channels/:id/summarize` | `model.use` | Auth() only | RequirePermission |
|
||||
| `POST /channels/:id/generate-title` | `model.use` | Auth() only | RequirePermission |
|
||||
| `POST /knowledge-bases/:id/search` | `kb.read` | Auth() only | RequirePermission |
|
||||
| `PUT /knowledge-bases/:id` | `kb.write` | Auth() only | RequirePermission |
|
||||
| `DELETE /knowledge-bases/:id` | `kb.write` | Auth() only | RequirePermission |
|
||||
| `DELETE /knowledge-bases/:id/documents/:docId` | `kb.write` | Auth() only | RequirePermission |
|
||||
| `POST /knowledge-bases/:id/rebuild` | `kb.write` | Auth() only | RequirePermission |
|
||||
|
||||
### Routes Already Correctly Enforced (no changes)
|
||||
|
||||
| Route | Permission |
|
||||
|-------|-----------|
|
||||
| `POST /channels` | `channel.create` |
|
||||
| `POST /channels/:id/participants` | `channel.invite` |
|
||||
| `POST /personas` | `persona.create` |
|
||||
| `PUT /personas/:id` | `persona.manage` |
|
||||
| `DELETE /personas/:id` | `persona.manage` |
|
||||
| `POST /knowledge-bases` | `kb.create` |
|
||||
| `POST /knowledge-bases/:id/documents` | `kb.write` |
|
||||
| `POST /workflows` | `workflow.create` |
|
||||
| `PATCH /workflows/:id` | `workflow.create` |
|
||||
| `DELETE /workflows/:id` | `workflow.create` |
|
||||
| All workflow stage routes | `workflow.create` |
|
||||
| `POST /tasks` | `task.create` |
|
||||
| `PUT /tasks/:id` | `task.create` |
|
||||
| `DELETE /tasks/:id` | `task.create` |
|
||||
| `POST /tasks/:id/run` | `task.create` |
|
||||
| `POST /tasks/:id/kill` | `task.create` |
|
||||
| Team tasks (scoped) | `task.create` |
|
||||
| All admin routes | `RequireAdmin()` |
|
||||
| Team-scoped routes | `RequireTeamAdmin()` |
|
||||
|
||||
### Routes Correctly Using Auth() Only (no permission needed)
|
||||
|
||||
Read-only data for the authenticated user: channels list, messages, notes,
|
||||
projects, profile, settings, models/enabled, notifications, extensions,
|
||||
groups/mine, teams/mine, presence, folders, workspaces, files, memories,
|
||||
packages, surfaces, usage, search, export/me.
|
||||
|
||||
### Everyone Group Default Permissions
|
||||
|
||||
The migration seeds Everyone with: `model.use`, `kb.read`, `channel.create`.
|
||||
|
||||
These three permissions mean that on a default deployment, the new
|
||||
enforcement on completions (`model.use`) and KB search (`kb.read`) will
|
||||
not change behavior — users already have these via Everyone.
|
||||
|
||||
The enforcement matters when an admin removes permissions from Everyone
|
||||
or creates restricted groups. Without these route-level gates, removing
|
||||
`model.use` from Everyone did nothing — users could still call completions.
|
||||
|
||||
### New Tests
|
||||
|
||||
12 test functions in `perm_enforcement_test.go`:
|
||||
|
||||
| Test | Permission | Pattern |
|
||||
|------|-----------|---------|
|
||||
| `TestPermEnforce_ModelUse_Completions` | `model.use` | deny → grant → allow |
|
||||
| `TestPermEnforce_ChannelCreate` | `channel.create` | deny → grant → allow |
|
||||
| `TestPermEnforce_ChannelInvite` | `channel.invite` | deny → grant → allow |
|
||||
| `TestPermEnforce_KBCreate` | `kb.create` | deny → grant → allow |
|
||||
| `TestPermEnforce_KBSearch` | `kb.read` | deny → grant → allow |
|
||||
| `TestPermEnforce_KBWrite` | `kb.write` | deny → grant → allow |
|
||||
| `TestPermEnforce_PersonaCreate` | `persona.create` | deny → grant → allow |
|
||||
| `TestPermEnforce_PersonaManage` | `persona.manage` | deny → grant → allow |
|
||||
| `TestPermEnforce_WorkflowCreate` | `workflow.create` | deny → grant → allow |
|
||||
| `TestPermEnforce_TaskCreate` | `task.create` | deny → grant → allow |
|
||||
| `TestPermEnforce_ProfilePermissions_NoGroups` | — | empty perms |
|
||||
| `TestPermEnforce_ProfilePermissions_WithGrant` | — | granted perms appear |
|
||||
| `TestPermEnforce_ProfilePermissions_Admin` | — | admin gets all |
|
||||
|
||||
### Not Covered (test harness limitation)
|
||||
|
||||
Summarize and GenerateTitle routes are not in the integration test harness
|
||||
(`setupHarness`). They share `model.use` with completions, which IS tested.
|
||||
Route-level wiring is applied in main.go. Full coverage comes when the
|
||||
test harness is extended to include these routes.
|
||||
Reference in New Issue
Block a user