|
| 1 | +# Binding and Credential Management |
| 2 | + |
| 3 | +## Overview |
| 4 | + |
| 5 | +Add full CRUD for bindings and credential value replacement via both CLI and REST API. |
| 6 | + |
| 7 | +Current gaps: |
| 8 | +- No standalone CLI for binding management (only implicit via `cred add --destination`) |
| 9 | +- No way to update a binding (must delete and recreate) |
| 10 | +- No way to replace a credential's value without deleting and re-adding (loses bindings) |
| 11 | +- No way to create multiple bindings per credential from CLI |
| 12 | +- API has `POST/GET/DELETE` for bindings but no `PATCH` |
| 13 | + |
| 14 | +## Context |
| 15 | + |
| 16 | +- `cmd/sluice/cred.go` -- credential CLI. Creates one binding per `cred add --destination`. Uses `store.AddRuleAndBinding()`. |
| 17 | +- `internal/store/store.go` -- store has: `AddBinding()`, `RemoveBinding()`, `ListBindings()`, `ListBindingsByCredential()`, `RemoveBindingsByCredential()`. No `UpdateBinding()`. |
| 18 | +- `internal/store/store.go:529` -- `BindingOpts` struct: Ports, Header, Template, Protocols. |
| 19 | +- `internal/vault/store.go` -- vault `Add()` overwrites existing credential (atomic temp+rename). Can be used for replacement. |
| 20 | +- `api/openapi.yaml` -- has `POST/GET/DELETE` for bindings, `POST/GET/DELETE` for credentials. No `PATCH` for either. |
| 21 | +- `internal/api/server.go` -- API handlers for bindings and credentials. |
| 22 | +- CLI pattern: `cmd/sluice/policy.go` for rules CRUD, `cmd/sluice/mcp.go` for MCP upstream CRUD. |
| 23 | + |
| 24 | +## Development Approach |
| 25 | + |
| 26 | +- **Testing approach**: Regular (code first, then tests) |
| 27 | +- **CRITICAL: every task MUST include new/updated tests** |
| 28 | +- **CRITICAL: all tests must pass before starting next task** |
| 29 | +- Run `go test ./... -timeout 30s` after each change |
| 30 | +- Maintain backward compatibility. Existing single `--destination` flag on `cred add` must keep working. |
| 31 | + |
| 32 | +## Testing Strategy |
| 33 | + |
| 34 | +- **Unit tests**: Go tests for CLI commands, store methods, API handlers |
| 35 | +- **E2e tests**: Deferred to manual testing |
| 36 | + |
| 37 | +## Progress Tracking |
| 38 | + |
| 39 | +- Mark completed items with `[x]` immediately when done |
| 40 | +- Add newly discovered tasks with + prefix |
| 41 | +- Document issues/blockers with ! prefix |
| 42 | + |
| 43 | +## Solution Overview |
| 44 | + |
| 45 | +### 1. Binding CRUD (CLI + API) |
| 46 | + |
| 47 | +```bash |
| 48 | +sluice binding add <credential> --destination <host> [--ports 443] [--header Authorization] [--template "Bearer {value}"] |
| 49 | +sluice binding list [--credential <name>] |
| 50 | +sluice binding update <id> [--destination <host>] [--ports 443] [--header Authorization] [--template "Bearer {value}"] |
| 51 | +sluice binding remove <id> |
| 52 | +``` |
| 53 | + |
| 54 | +API additions: `PATCH /api/bindings/{id}` for updates. |
| 55 | + |
| 56 | +### 2. Multi-binding on `cred add` |
| 57 | + |
| 58 | +Instead of complex flag grouping (Go's flag package does not support positional grouping of repeatable flags), use a simple repeated `--destination` flag. Each destination gets the same ports/header/template from the shared flags. For per-destination customization, use `sluice binding add` after creation. |
| 59 | + |
| 60 | +```bash |
| 61 | +# Multiple destinations, same options |
| 62 | +sluice cred add github_pat \ |
| 63 | + --destination api.github.com \ |
| 64 | + --destination uploads.github.com \ |
| 65 | + --ports 443 --header Authorization --template "Bearer {value}" |
| 66 | + |
| 67 | +# Per-destination customization via binding add |
| 68 | +sluice cred add mykey |
| 69 | +sluice binding add mykey --destination api.example.com --ports 443 --header X-API-Key |
| 70 | +sluice binding add mykey --destination other.example.com --ports 8080 --header Authorization |
| 71 | +``` |
| 72 | + |
| 73 | +Each `--destination` creates both an allow rule and a binding (matching current `cred add` behavior). |
| 74 | + |
| 75 | +### 3. Credential value replacement |
| 76 | + |
| 77 | +```bash |
| 78 | +# Replace credential value (prompts for new value, never shows current) |
| 79 | +sluice cred update <name> |
| 80 | +``` |
| 81 | + |
| 82 | +API addition: `PATCH /api/credentials/{name}` with new value in request body. |
| 83 | + |
| 84 | +The vault's `Add()` already does atomic overwrite (temp file + rename). For OAuth credentials, `cred update` prompts for new access/refresh tokens and rebuilds the JSON blob. Bindings and rules are preserved. |
| 85 | + |
| 86 | +### Important notes |
| 87 | + |
| 88 | +- CLI commands write to the DB/vault only. Changes take effect in the running proxy on SIGHUP, consistent with `policy add` and `cred add`. |
| 89 | +- `binding add` does not validate credential exists in vault (consistent with existing behavior, avoids vault I/O overhead for remote providers). |
| 90 | +- Credential values are never displayed. `cred update` only writes, never reads. |
| 91 | + |
| 92 | +## What Goes Where |
| 93 | + |
| 94 | +- **Implementation Steps**: store methods, CLI commands, API endpoints, tests |
| 95 | +- **Post-Completion**: manual testing with running proxy |
| 96 | + |
| 97 | +## Implementation Steps |
| 98 | + |
| 99 | +### Task 1: Add `UpdateBinding` to store |
| 100 | + |
| 101 | +**Files:** |
| 102 | +- Modify: `internal/store/store.go` |
| 103 | + |
| 104 | +- [ ] Add `UpdateBinding(id int64, opts BindingUpdateOpts) error` method. `BindingUpdateOpts` has optional fields (Destination, Ports, Header, Template, Protocols) using pointer-nil-means-skip pattern (same as `ChannelUpdate`). |
| 105 | +- [ ] Write tests for UpdateBinding (update single field, update multiple fields, not found) |
| 106 | +- [ ] Run tests: `go test ./internal/store/ -timeout 30s` |
| 107 | + |
| 108 | +### Task 2: Add `sluice binding` CLI subcommand |
| 109 | + |
| 110 | +**Files:** |
| 111 | +- Create: `cmd/sluice/binding.go` |
| 112 | +- Modify: `cmd/sluice/main.go` |
| 113 | + |
| 114 | +- [ ] Add `case "binding"` dispatch in `cmd/sluice/main.go` |
| 115 | +- [ ] Create `cmd/sluice/binding.go` with subcommand registration |
| 116 | +- [ ] Implement `sluice binding add <credential> --destination <host> [--ports 443] [--header Authorization] [--template "Bearer {value}"]`: calls `store.AddBinding()`, also creates allow rule for the destination |
| 117 | +- [ ] Implement `sluice binding list [--credential <name>]`: calls `store.ListBindings()` or `store.ListBindingsByCredential()`, prints formatted output (ID, credential, destination, ports, header, template) |
| 118 | +- [ ] Implement `sluice binding update <id> [--destination <host>] [--ports 443] [--header Authorization] [--template "Bearer {value}"]`: calls `store.UpdateBinding()`, only updates provided flags |
| 119 | +- [ ] Implement `sluice binding remove <id>`: calls `store.RemoveBinding()` |
| 120 | +- [ ] Write tests for add (success, missing args) |
| 121 | +- [ ] Write tests for list (all, filtered by credential) |
| 122 | +- [ ] Write tests for update (single field, multiple fields, not found) |
| 123 | +- [ ] Write tests for remove (success, not found) |
| 124 | +- [ ] Run tests: `go test ./cmd/sluice/ -timeout 30s` |
| 125 | + |
| 126 | +### Task 3: Support multiple `--destination` on `cred add` |
| 127 | + |
| 128 | +**Files:** |
| 129 | +- Modify: `cmd/sluice/cred.go` |
| 130 | + |
| 131 | +- [ ] Change `--destination` from single string to repeatable string slice flag |
| 132 | +- [ ] When multiple destinations provided: create one credential in vault, then call `AddRuleAndBinding()` for each destination (each gets the same ports/header/template from shared flags) |
| 133 | +- [ ] Maintain backward compatibility: single `--destination` still works as before |
| 134 | +- [ ] Write tests for single destination (backward compat) |
| 135 | +- [ ] Write tests for multiple destinations |
| 136 | +- [ ] Write tests for error cases (no destination provided still works for credential-only add) |
| 137 | +- [ ] Run tests: `go test ./cmd/sluice/ -timeout 30s` |
| 138 | + |
| 139 | +### Task 4: Add `sluice cred update` for value replacement |
| 140 | + |
| 141 | +**Files:** |
| 142 | +- Modify: `cmd/sluice/cred.go` |
| 143 | + |
| 144 | +- [ ] Implement `sluice cred update <name>`: verify credential exists via `vault.List()`, prompt for new value via stdin/terminal (never show current value), call `vault.Add()` to overwrite |
| 145 | +- [ ] For OAuth credentials (detected via `IsOAuth()`): prompt for new access token and optionally refresh token, rebuild OAuth JSON blob, overwrite in vault, regenerate phantom files |
| 146 | +- [ ] Print confirmation message after successful update |
| 147 | +- [ ] Write tests for static credential update |
| 148 | +- [ ] Write tests for OAuth credential update (access only, access + refresh) |
| 149 | +- [ ] Write tests for update of nonexistent credential |
| 150 | +- [ ] Run tests: `go test ./cmd/sluice/ -timeout 30s` |
| 151 | + |
| 152 | +### Task 5: API endpoints for binding update and credential update |
| 153 | + |
| 154 | +**Files:** |
| 155 | +- Modify: `api/openapi.yaml` |
| 156 | +- Modify: `internal/api/server.go` |
| 157 | +- Modify: `internal/api/api.gen.go` (regenerated) |
| 158 | + |
| 159 | +- [ ] Add `PATCH /api/bindings/{id}` to OpenAPI spec (request: BindingUpdate with optional destination, ports, header, template; response: Binding) |
| 160 | +- [ ] Add `PATCH /api/credentials/{name}` to OpenAPI spec (request: new value; response: 204). For OAuth type, request body includes access_token and optional refresh_token. |
| 161 | +- [ ] Regenerate API code: `go generate ./internal/api/` |
| 162 | +- [ ] Implement `PatchApiBindingsId` handler: validate input, call `store.UpdateBinding()` |
| 163 | +- [ ] Implement `PatchApiCredentialsName` handler: validate credential exists, call `vault.Add()` to overwrite. For OAuth: rebuild JSON blob. Regenerate phantom files. |
| 164 | +- [ ] Write tests for PATCH /api/bindings/{id} (success, not found, partial update) |
| 165 | +- [ ] Write tests for PATCH /api/credentials/{name} (static, OAuth, not found) |
| 166 | +- [ ] Run tests: `go test ./internal/api/ -timeout 30s` |
| 167 | + |
| 168 | +### Task 6: Verify acceptance criteria |
| 169 | + |
| 170 | +- [ ] Verify all binding CRUD operations work via CLI |
| 171 | +- [ ] Verify all binding CRUD operations work via API |
| 172 | +- [ ] Verify `cred add` with multiple `--destination` flags creates multiple bindings |
| 173 | +- [ ] Verify `cred update` replaces value without affecting bindings |
| 174 | +- [ ] Verify `cred update` works for OAuth credentials |
| 175 | +- [ ] Verify existing single `--destination` behavior unchanged |
| 176 | +- [ ] Run full test suite: `go test ./... -v -timeout 30s` |
| 177 | + |
| 178 | +### Task 7: [Final] Update documentation |
| 179 | + |
| 180 | +- [ ] Update CLAUDE.md CLI subcommands with `sluice binding` and `sluice cred update` |
| 181 | +- [ ] Move this plan to `docs/plans/completed/` |
| 182 | + |
| 183 | +## Post-Completion |
| 184 | + |
| 185 | +**Manual verification:** |
| 186 | +- Create credential with 3 destinations in one command |
| 187 | +- Add a 4th binding via `sluice binding add` |
| 188 | +- Update a binding's header via `sluice binding update` |
| 189 | +- Remove one binding, verify credential and other bindings remain |
| 190 | +- Replace credential value via `sluice cred update`, verify proxy uses new value after SIGHUP |
| 191 | +- Test PATCH endpoints via curl |
0 commit comments