feat: Security-as-Code for Provider Management (Closes #11)#53
Merged
Conversation
- Add audit.Service to record control-plane mutations - Wire AuditService to ProvidersHandler and CallbackHandler - Add GET /v1/audit endpoint to query audit events - Add created_at index to audit_events table for fast querying - Build nexus-cli declarative reconciler tool for managing providers via YAML - Implement Terraform-style plan/apply/prune workflow in nexus-cli
- Change AuditEvent fields to *string to fix nullable scan bug - Add --prune flag to nexus-cli to prevent accidental deletions - Remove dead code and add TrimSpace in IP extraction logic - Add GitHub Actions CI/CD workflow for nexus-cli plan/apply - Verified STATE_KEY fatal-exit guard is present in both broker and gateway
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
- Add docs/guides/security-as-code.md — full nexus-cli guide covering plan/apply workflow, manifest format, --prune flag, and CI/CD setup - Add docs/reference/audit-log.md — GET /v1/audit endpoint reference with event types, query params, and response schema - Update docs/services/broker.md — add Audit Subsystem section (§5) and clarify STATE_KEY fatal-exit in the env vars table - Update docs/reference/security-model.md — add Audit Trail and STATE_KEY Startup Guard sections - Update docs/guides/managing-providers.md — add tip callout pointing to the declarative nexus-cli workflow - Update mkdocs.yml — wire new pages into nav
Contributor
There was a problem hiding this comment.
Pull request overview
Implements the “Security-as-Code for Provider Management” epic by adding centralized audit logging in the Broker (plus an audit query endpoint/index), and introducing a declarative nexus-cli with a GitHub Actions workflow to reconcile provider state from a YAML manifest.
Changes:
- Added
internal/audit.Serviceand wired it into provider mutations and OAuth callback handling. - Added
GET /auditBroker endpoint withevent_type,since(RFC3339), andlimitfilters; added a DB index onaudit_events.created_at. - Introduced
nexus-cli plan/applywith env-var expansion for manifests, an examplenexus-providers.yaml, and CI automation.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
nexus-cli/nexus-providers.yaml |
Adds an example declarative provider manifest. |
nexus-cli/main.go |
Implements plan/apply reconciliation logic against the Broker provider endpoints. |
nexus-cli/go.mod |
Introduces a separate Go module for the CLI. |
nexus-cli/go.sum |
Adds checksum entries for CLI dependencies. |
nexus-cli/.gitignore |
Ignores the built CLI binary in the CLI module directory. |
nexus-broker/pkg/storage/pg.go |
Adjusts AuditEvent fields to pointers to better represent nullable DB columns. |
nexus-broker/pkg/handlers/providers.go |
Injects audit logging for provider create/update/delete operations. |
nexus-broker/pkg/handlers/providers_test.go |
Updates handler construction to match the new constructor signature. |
nexus-broker/pkg/handlers/callback.go |
Refactors inline audit insertion to use the shared audit service. |
nexus-broker/pkg/handlers/audit.go |
Adds an audit query handler with filtering and pagination. |
nexus-broker/migrations/11_add_audit_created_at_index.sql |
Adds an index to speed created_at DESC audit queries. |
nexus-broker/internal/audit/service.go |
Adds centralized audit event writer (IP/User-Agent extraction + JSON event_data). |
nexus-broker/cmd/nexus-broker/main.go |
Wires audit service + audit handler into the Broker router. |
.github/workflows/nexus-cli.yml |
Adds CI workflow to run plan on PRs and apply --prune on merges to main. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| log.Fatalf("Failed to create request: %v", err) | ||
| } | ||
| if apiKey != "" { | ||
| req.Header.Set("Authorization", "Bearer "+apiKey) |
Comment on lines
+99
to
+103
| client := &http.Client{} | ||
| resp, err := client.Do(req) | ||
| if err != nil { | ||
| log.Fatalf("Failed to fetch live providers: %v", err) | ||
| } |
Comment on lines
+137
to
+145
| for _, p := range manifest.Providers { | ||
| if id, exists := liveProviderMap[p.Name]; exists { | ||
| toUpdate[id] = p | ||
| fmt.Printf("~ UPDATE : %s\n", p.Name) | ||
| } else { | ||
| toCreate = append(toCreate, p) | ||
| fmt.Printf("+ CREATE : %s\n", p.Name) | ||
| } | ||
| } |
| @@ -0,0 +1,5 @@ | |||
| module github.com/Prescott-Data/nexus-framework/nexus-cli | |||
|
|
|||
| go 1.26.2 | |||
Comment on lines
+36
to
+41
| if: github.event_name == 'pull_request' | ||
| env: | ||
| BROKER_BASE_URL: ${{ secrets.BROKER_BASE_URL }} | ||
| API_KEY: ${{ secrets.BROKER_API_KEY }} | ||
| run: ./nexus-cli plan | ||
|
|
Comment on lines
+29
to
+35
| ipVal = &ip | ||
| } else { | ||
| host, _, err := net.SplitHostPort(r.RemoteAddr) | ||
| if err == nil { | ||
| ipVal = &host | ||
| } else { | ||
| ipVal = &r.RemoteAddr |
Comment on lines
+66
to
+68
| if h.audit != nil { | ||
| _ = h.audit.Log("provider.updated", nil, map[string]interface{}{"provider_id": profile.ID, "name": profile.Name}, r) | ||
| } |
Comment on lines
93
to
98
| protected := router.With( | ||
| server.ApiKeyMiddleware(cfg.RequireAPIKey, cfg.APIKeys), | ||
| server.AllowlistMiddleware(cfg.RequireAllowlist, cfg.AllowedCIDRs), | ||
| ) | ||
| protected.Get("/audit", auditHandler.List) | ||
| protected.Route("/providers", func(r chi.Router) { |
Comment on lines
+23
to
+35
| // List handles GET /audit to retrieve recent audit events | ||
| func (h *AuditHandler) List(w http.ResponseWriter, r *http.Request) { | ||
| eventType := r.URL.Query().Get("event_type") | ||
| sinceStr := r.URL.Query().Get("since") | ||
| limitStr := r.URL.Query().Get("limit") | ||
|
|
||
| limit := 50 | ||
| if limitStr != "" { | ||
| if parsedLimit, err := strconv.Atoi(limitStr); err == nil && parsedLimit > 0 && parsedLimit <= 1000 { | ||
| limit = parsedLimit | ||
| } | ||
| } | ||
|
|
Comment on lines
+837
to
848
| if h.audit == nil { | ||
| return | ||
| } | ||
|
|
||
| // Convert map[string]string to map[string]interface{} for the audit service | ||
| auditData := make(map[string]interface{}) | ||
| for k, v := range data { | ||
| auditData[k] = v | ||
| } | ||
|
|
||
| _, _ = h.db.Exec(` | ||
| INSERT INTO audit_events (connection_id, event_type, event_data, ip_address, user_agent) | ||
| VALUES ($1, $2, $3, $4, $5)`, | ||
| connectionID, eventType, string(eventData), ipVal, r.Header.Get("User-Agent")) | ||
| _ = h.audit.Log(eventType, connectionID, auditData, r) | ||
| } |
- nexus-cli: use X-API-Key header (was Authorization: Bearer) - nexus-cli: add 30s timeout to shared http.Client - nexus-cli: diff-based update detection via providerDrifted() — skip no-op UPDATEs and print '= OK' for providers with no changes - nexus-cli/go.mod: align go directive to 1.21 (matches CI) - .github/workflows/nexus-cli.yml: guard plan step for forked PRs where secrets are unavailable (no-op with info message instead of failing) - audit/service.go: validate extracted IP with net.ParseIP before storing to prevent INSERT failures on arbitrary X-Forwarded-For values - handlers/providers.go: log audit errors with log.Printf instead of discarding with _ = - handlers/callback.go: log audit errors in logAuditEvent wrapper - handlers/audit_test.go: add unit tests for AuditHandler.List covering no filters, event_type filter, invalid since param (400), custom limit, and out-of-range limit clamping - docs/reference/audit-log.md: correct endpoint path to /audit (no /v1 prefix) with note explaining the Broker is currently unversioned
The secrets context is not valid in if: expressions in GitHub Actions. Replace with github.event.pull_request.head.repo.full_name == github.repository which is the correct pattern for detecting forked vs same-repo PRs.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated 13 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+120
to
+133
| var liveProviders []map[string]interface{} | ||
| if err := json.NewDecoder(resp.Body).Decode(&liveProviders); err != nil { | ||
| log.Fatalf("Failed to decode live providers: %v", err) | ||
| } | ||
|
|
||
| // Build live provider map: name → {id, full config} | ||
| liveProviderMap := make(map[string]map[string]interface{}) | ||
| for _, lp := range liveProviders { | ||
| name, nameOk := lp["name"].(string) | ||
| _, idOk := lp["id"].(string) | ||
| if nameOk && idOk { | ||
| liveProviderMap[name] = lp | ||
| } | ||
| } |
Comment on lines
+92
to
+98
| // Expand environment variables | ||
| expandedData := os.ExpandEnv(string(data)) | ||
|
|
||
| var manifest Manifest | ||
| if err := yaml.Unmarshal([]byte(expandedData), &manifest); err != nil { | ||
| log.Fatalf("Failed to parse YAML manifest: %v", err) | ||
| } |
Comment on lines
+235
to
+251
| for id, p := range toUpdate { | ||
| fmt.Printf("Updating %s... ", p.Name) | ||
|
|
||
| jsonData, err := json.Marshal(p) | ||
| if err != nil { | ||
| fmt.Printf("Failed to marshal: %v\n", err) | ||
| continue | ||
| } | ||
|
|
||
| req, err := http.NewRequest("PUT", brokerURL+"/providers/"+id, bytes.NewBuffer(jsonData)) | ||
| if err != nil { | ||
| fmt.Printf("Failed to create request: %v\n", err) | ||
| continue | ||
| } | ||
| setAPIKey(req, apiKey) | ||
| req.Header.Set("Content-Type", "application/json") | ||
|
|
|
|
||
| var eventDataJSON []byte | ||
| if data != nil { | ||
| eventDataJSON, _ = json.Marshal(data) |
|
|
||
| if h.audit != nil { | ||
| if err := h.audit.Log("provider.updated", nil, map[string]interface{}{"provider_id": profile.ID, "name": profile.Name}, r); err != nil { | ||
| log.Printf("audit: failed to log provider.updated for provider_id=%s: %v", profile.ID, err) |
|
|
||
| `nexus-cli` follows a **plan → confirm → apply** workflow: | ||
|
|
||
| 1. **Fetches** the current live state from `GET /v1/providers`. |
Comment on lines
+11
to
+18
| | Event Type | Trigger | | ||
| | :--- | :--- | | ||
| | `provider.created` | A new provider profile is registered | | ||
| | `provider.updated` | A provider's configuration is modified (`PUT` or `PATCH`) | | ||
| | `provider.deleted` | A provider is deleted (by ID or by name) | | ||
| | `connection.created` | An OAuth callback completes successfully and a connection is established | | ||
| | `connection.revoked` | A connection is explicitly revoked | | ||
|
|
Comment on lines
+32
to
+36
| Every control-plane mutation is recorded in the `audit_events` table via the `audit.Service`: | ||
| - **`provider.created`** — logged on every successful `POST /providers` call. | ||
| - **`provider.updated`** — logged on `PUT` and `PATCH` mutations. | ||
| - **`provider.deleted`** — logged on deletion by ID or by name. | ||
| - **`connection.created`** — logged on every successful OAuth callback. |
| Both the Broker and Gateway will **fatal-exit at startup** if the `STATE_KEY` environment variable is absent: | ||
|
|
||
| ``` | ||
| FATAL: STATE_KEY environment variable is required and must be identical across Broker and Gateway |
|
|
||
| go 1.21 | ||
|
|
||
| require gopkg.in/yaml.v3 v3.0.1 // indirect |
The Broker's GET /providers endpoint only returns ID and Name, causing the CLI
to incorrectly report drift for all providers even when no fields had changed.
The CLI now iterates over the list and fetches the full profile via
GET /providers/{id} so that providerDrifted() can perform an accurate comparison.
Replaced os.ExpandEnv with a custom os.Expand callback that detects unset environment variables. This prevents them from being quietly replaced with empty strings, which would cause omitempty fields (like client_secret) to be dropped from the payload and written as NULL to the database by the Broker, effectively wiping the credentials.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+96
to
+100
| if h.audit != nil { | ||
| if err := h.audit.Log("provider.updated", nil, map[string]interface{}{"provider_id": id.String(), "updates": updates}, r); err != nil { | ||
| log.Printf("audit: failed to log provider.updated for provider_id=%s: %v", id, err) | ||
| } | ||
| } |
Comment on lines
+46
to
+49
| var eventDataJSON []byte | ||
| if data != nil { | ||
| eventDataJSON, _ = json.Marshal(data) | ||
| } |
Comment on lines
93
to
99
| protected := router.With( | ||
| server.ApiKeyMiddleware(cfg.RequireAPIKey, cfg.APIKeys), | ||
| server.AllowlistMiddleware(cfg.RequireAllowlist, cfg.AllowedCIDRs), | ||
| ) | ||
| protected.Get("/audit", auditHandler.List) | ||
| protected.Route("/providers", func(r chi.Router) { | ||
| r.Post("/", providersHandler.Register) |
| - **`provider.created`** — logged on every successful `POST /providers` call. | ||
| - **`provider.updated`** — logged on `PUT` and `PATCH` mutations. | ||
| - **`provider.deleted`** — logged on deletion by ID or by name. | ||
| - **`connection.created`** — logged on every successful OAuth callback. |
Comment on lines
+16
to
+17
| | `connection.created` | An OAuth callback completes successfully and a connection is established | | ||
| | `connection.revoked` | A connection is explicitly revoked | |
| - **Structured event data** (provider ID, name, workspace ID) | ||
| - The **caller IP address** and **User-Agent** | ||
|
|
||
| This audit log is queryable via the [`GET /v1/audit`](audit-log.md) endpoint and is the foundational building block for compliance, forensic analysis, and detecting unauthorized mutations. |
Comment on lines
+14
to
+18
| 1. **Fetches** the current live state from `GET /v1/providers`. | ||
| 2. **Diffs** it against your `nexus-providers.yaml` manifest. | ||
| 3. **Prints** a human-readable plan showing creates, updates, and orphaned providers. | ||
| 4. **Applies** the changes only after you confirm with `yes` (or non-interactively in CI). | ||
|
|
Comment on lines
+141
to
+174
| // Build live provider map: name → {id, full config} | ||
| liveProviderMap := make(map[string]map[string]interface{}) | ||
| for _, lp := range liveProviders { | ||
| name, nameOk := lp["name"].(string) | ||
| id, idOk := lp["id"].(string) | ||
| if nameOk && idOk { | ||
| // Fetch full profile for accurate drift detection | ||
| reqProfile, err := http.NewRequest("GET", brokerURL+"/providers/"+id, nil) | ||
| if err != nil { | ||
| log.Fatalf("Failed to create request for provider %s: %v", name, err) | ||
| } | ||
| setAPIKey(reqProfile, apiKey) | ||
|
|
||
| respProfile, err := httpClient.Do(reqProfile) | ||
| if err != nil { | ||
| log.Fatalf("Failed to fetch profile for provider %s: %v", name, err) | ||
| } | ||
|
|
||
| if respProfile.StatusCode != http.StatusOK { | ||
| body, _ := io.ReadAll(respProfile.Body) | ||
| respProfile.Body.Close() | ||
| log.Fatalf("Failed to fetch profile for %s, status: %d, body: %s", name, respProfile.StatusCode, string(body)) | ||
| } | ||
|
|
||
| var fullProfile map[string]interface{} | ||
| if err := json.NewDecoder(respProfile.Body).Decode(&fullProfile); err != nil { | ||
| respProfile.Body.Close() | ||
| log.Fatalf("Failed to decode profile for %s: %v", name, err) | ||
| } | ||
| respProfile.Body.Close() | ||
|
|
||
| liveProviderMap[name] = fullProfile | ||
| } | ||
| } |
Comment on lines
+181
to
+200
| fmt.Println("\n--- Execution Plan ---") | ||
|
|
||
| toCreate := []Provider{} | ||
| toUpdate := make(map[string]Provider) // map ID to Provider | ||
| toDelete := []string{} // list of IDs | ||
| toDeleteNames := []string{} | ||
|
|
||
| for _, p := range manifest.Providers { | ||
| if live, exists := liveProviderMap[p.Name]; exists { | ||
| id := live["id"].(string) | ||
| if providerDrifted(p, live) { | ||
| toUpdate[id] = p | ||
| fmt.Printf("~ UPDATE : %s\n", p.Name) | ||
| } else { | ||
| fmt.Printf("= OK : %s (no changes)\n", p.Name) | ||
| } | ||
| } else { | ||
| toCreate = append(toCreate, p) | ||
| fmt.Printf("+ CREATE : %s\n", p.Name) | ||
| } |
|
|
||
| go 1.21 | ||
|
|
||
| require gopkg.in/yaml.v3 v3.0.1 // indirect |
As noted by Copilot, using PUT with a struct-marshaled payload would overwrite unspecified/omitted fields with empty strings, causing the Broker to null out existing credentials and configurations. Refactored the CLI's apply logic to compute a JSON diff against the full live profile, sending only the modified fields via a PATCH request.
If event data fails to marshal, the error is now returned to callers rather than inserting a row with NULL event_data and losing audit context.
The docs listed connection.created and connection.revoked, but the callback handler emits oauth_flow_completed, token_exchange_failed, token_retrieved, token_refresh_fatal, oauth_error, etc. Updated both broker.md and audit-log.md to reflect the real event type strings.
Replaces the sequential N+1 fetch loop with a goroutine pool (max 5 concurrent requests) using a semaphore channel. This keeps plan/apply responsive as the provider count grows, without requiring a new bulk endpoint on the Broker.
The plan now prints each changed field with old → new values when drift is detected, making it clear exactly what will change before applying. Sensitive fields (client_secret, client_id) are masked with *** to prevent secret leakage in CI logs.
gopkg.in/yaml.v3 is directly imported in main.go but was marked as // indirect in go.mod. Running go mod tidy corrects this.
The manifest uses ${GOOGLE_CLIENT_ID}, ${GITHUB_CLIENT_ID}, etc. but
the workflow only passed BROKER_BASE_URL and API_KEY, causing the
fail-fast env var guard to abort. Added all four provider credential
secrets to both the Plan and Apply step env blocks.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 22 changed files in this pull request and generated 14 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+67
to
+70
| if h.audit != nil { | ||
| if err := h.audit.Log("provider.updated", nil, map[string]interface{}{"provider_id": profile.ID, "name": profile.Name}, r); err != nil { | ||
| log.Printf("audit: failed to log provider.updated for provider_id=%s: %v", profile.ID, err) | ||
| } |
Comment on lines
+96
to
+99
| if h.audit != nil { | ||
| if err := h.audit.Log("provider.updated", nil, map[string]interface{}{"provider_id": id.String(), "updates": updates}, r); err != nil { | ||
| log.Printf("audit: failed to log provider.updated for provider_id=%s: %v", id, err) | ||
| } |
Comment on lines
+119
to
+122
| if h.audit != nil { | ||
| if err := h.audit.Log("provider.deleted", nil, map[string]interface{}{"provider_id": id.String()}, r); err != nil { | ||
| log.Printf("audit: failed to log provider.deleted for provider_id=%s: %v", id, err) | ||
| } |
|
|
||
| if h.audit != nil { | ||
| if err := h.audit.Log("provider.created", nil, map[string]interface{}{"provider_id": profile.ID, "name": profile.Name}, r); err != nil { | ||
| log.Printf("audit: failed to log provider.created for provider_id=%s: %v", profile.ID, err) |
|
|
||
| for _, p := range manifest.Providers { | ||
| if live, exists := liveProviderMap[p.Name]; exists { | ||
| id := live["id"].(string) |
Comment on lines
+14
to
+19
| // Marshal and unmarshal desired to get a map reflecting exactly what | ||
| // would be sent as JSON (respecting json tags and omitempty). | ||
| b, _ := json.Marshal(desired) | ||
| var desiredMap map[string]interface{} | ||
| _ = json.Unmarshal(b, &desiredMap) | ||
|
|
Comment on lines
+37
to
+42
|
|
||
| // Special case: if desired is an empty string and live is missing/null | ||
| if string(vb) == `""` && (string(lvb) == "null" || string(lvb) == "") { | ||
| continue | ||
| } | ||
|
|
Comment on lines
+148
to
+159
| func TestAuditList_LimitAboveMax_ClampedTo1000(t *testing.T) { | ||
| db, mock := newSqlxDB(t) | ||
| defer db.Close() | ||
|
|
||
| rows := sqlmock.NewRows([]string{ | ||
| "id", "connection_id", "event_type", "event_data", "ip_address", "user_agent", "created_at", | ||
| }) | ||
|
|
||
| // limit=9999 should be clamped to 50 (default, since > 1000 is rejected) | ||
| mock.ExpectQuery(`SELECT id, connection_id, event_type, event_data, ip_address, user_agent, created_at`). | ||
| WithArgs(50). | ||
| WillReturnRows(rows) |
Comment on lines
93
to
98
| protected := router.With( | ||
| server.ApiKeyMiddleware(cfg.RequireAPIKey, cfg.APIKeys), | ||
| server.AllowlistMiddleware(cfg.RequireAllowlist, cfg.AllowedCIDRs), | ||
| ) | ||
| protected.Get("/audit", auditHandler.List) | ||
| protected.Route("/providers", func(r chi.Router) { |
Comment on lines
+74
to
+93
| ```json | ||
| [ | ||
| { | ||
| "id": "a1b2c3d4-...", | ||
| "connection_id": "f5e6d7c8-...", | ||
| "event_type": "oauth_flow_completed", | ||
| "event_data": "{\"provider_id\": \"...\", \"workspace_id\": \"ws-123\"}", | ||
| "ip_address": "10.0.0.1", | ||
| "user_agent": "nexus-gateway/1.0", | ||
| "created_at": "2026-05-05T10:30:00Z" | ||
| }, | ||
| { | ||
| "id": "b2c3d4e5-...", | ||
| "connection_id": null, | ||
| "event_type": "provider.deleted", | ||
| "event_data": "{\"provider_id\": \"...\", \"provider_name\": \"old-slack\"}", | ||
| "ip_address": "192.168.1.5", | ||
| "user_agent": "curl/7.88.1", | ||
| "created_at": "2026-05-05T09:15:00Z" | ||
| } |
The nexus-cli workflow belongs in the internal repo, not the open-source framework. The CLI is a standalone operational tool — users run it from their own environments against whichever Broker they target. Updated security-as-code.md to show CI/CD as optional with a generic snippet instead of referencing a shipped workflow file.
- providers.go: fix %s format verb with uuid.UUID → %v (4 sites) - main.go: use checked type assertions for live provider IDs (2 sites) - main.go: read and print response body on create/update/delete failures - drift.go: return errors from json.Marshal/Unmarshal instead of discarding - drift.go: fix mixed spaces/tabs indentation (gofmt) - audit_test.go: rename ClampedTo1000 → FallsBackToDefault to match behavior - audit-log.md: fix response example to reflect omitempty (omit null fields) - audit-log.md: add note explaining omitempty behavior
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+143
to
+147
| type providerResult struct { | ||
| Name string | ||
| Profile map[string]interface{} | ||
| } | ||
|
|
Comment on lines
+96
to
+100
| if h.audit != nil { | ||
| if err := h.audit.Log("provider.updated", nil, map[string]interface{}{"provider_id": id.String(), "updates": updates}, r); err != nil { | ||
| log.Printf("audit: failed to log provider.updated for provider_id=%v: %v", id, err) | ||
| } | ||
| } |
Comment on lines
94
to
98
| server.ApiKeyMiddleware(cfg.RequireAPIKey, cfg.APIKeys), | ||
| server.AllowlistMiddleware(cfg.RequireAllowlist, cfg.AllowedCIDRs), | ||
| ) | ||
| protected.Get("/audit", auditHandler.List) | ||
| protected.Route("/providers", func(r chi.Router) { |
Comment on lines
87
to
92
| func TestRegisterProvider_Success(t *testing.T) { | ||
| // 1. Mocks the provider.Store. | ||
| mockStore := new(MockStore) | ||
| handler := NewProvidersHandler(mockStore) | ||
| handler := NewProvidersHandler(mockStore, nil) | ||
|
|
||
| // 2. Mocks the store.RegisterProfile method to return a valid Profile. |
Comment on lines
+172
to
+180
| ## CI/CD Integration (Optional) | ||
|
|
||
| `nexus-cli` is a standalone binary — you can run it from your laptop, a bastion host, or a CI pipeline. If you want to integrate it into your own CI/CD, here's a recommended pattern: | ||
|
|
||
| - **On pull requests**: run `nexus-cli plan` as an informational check so reviewers can see what would change. | ||
| - **Apply manually**: use a `workflow_dispatch` trigger or run `nexus-cli apply` from a trusted environment when you're ready. | ||
|
|
||
| > **Note:** Auto-applying on merge is discouraged. Provider configurations are live operational data — you should always review a plan before applying. | ||
|
|
- Remove unused providerResult type from CLI (compile error) - Introduce audit.Logger interface for testability - Redact client_secret/client_id in PATCH audit log to prevent persisting secrets in audit_events table - Add TestRegisterProvider_AuditsCreation: verifies provider.created event is logged on successful registration - Add TestPatchProvider_AuditRedactsSecrets: verifies client_secret is replaced with [REDACTED] before audit logging Items 3 and 5 (PR description mentioning /v1/audit and shipped workflow) are PR description fixes to be updated on GitHub.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 22 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+236
to
+244
| fmt.Printf("~ UPDATE : %s\n", p.Name) | ||
| for field, newVal := range updates { | ||
| oldVal := live[field] | ||
| if isSecretField(field) { | ||
| fmt.Printf(" %s: *** → ***\n", field) | ||
| } else { | ||
| fmt.Printf(" %s: %v → %v\n", field, formatVal(oldVal), formatVal(newVal)) | ||
| } | ||
| } |
Comment on lines
+295
to
+329
| for _, p := range toCreate { | ||
| fmt.Printf("Creating %s... ", p.Name) | ||
|
|
||
| payload := map[string]interface{}{ | ||
| "profile": p, | ||
| } | ||
|
|
||
| jsonData, err := json.Marshal(payload) | ||
| if err != nil { | ||
| fmt.Printf("Failed to marshal: %v\n", err) | ||
| continue | ||
| } | ||
|
|
||
| req, err := http.NewRequest("POST", brokerURL+"/providers", bytes.NewBuffer(jsonData)) | ||
| if err != nil { | ||
| fmt.Printf("Failed to create request: %v\n", err) | ||
| continue | ||
| } | ||
| setAPIKey(req, apiKey) | ||
| req.Header.Set("Content-Type", "application/json") | ||
|
|
||
| resp, err := httpClient.Do(req) | ||
| if err != nil { | ||
| fmt.Printf("Request failed: %v\n", err) | ||
| continue | ||
| } | ||
|
|
||
| if resp.StatusCode == http.StatusCreated || resp.StatusCode == http.StatusOK { | ||
| resp.Body.Close() | ||
| fmt.Println("OK") | ||
| } else { | ||
| errBody, _ := io.ReadAll(resp.Body) | ||
| resp.Body.Close() | ||
| fmt.Printf("FAILED (Status %d): %s\n", resp.StatusCode, string(errBody)) | ||
| } |
| } | ||
|
|
||
| if h.audit != nil { | ||
| if err := h.audit.Log("provider.updated", nil, map[string]interface{}{"provider_id": profile.ID, "name": profile.Name}, r); err != nil { |
Comment on lines
+231
to
+238
| func TestPatchProvider_AuditRedactsSecrets(t *testing.T) { | ||
| mockStore := new(MockStore) | ||
| mockAudit := new(MockAuditLogger) | ||
| handler := NewProvidersHandler(mockStore, mockAudit) | ||
|
|
||
| testID := uuid.New() | ||
| mockStore.On("PatchProfile", testID, mock.AnythingOfType("map[string]interface {}")).Return(nil) | ||
| mockAudit.On("Log", "provider.updated", (*uuid.UUID)(nil), mock.AnythingOfType("map[string]interface {}"), mock.AnythingOfType("*http.Request")).Return(nil) |
| rows := sqlmock.NewRows([]string{ | ||
| "id", "connection_id", "event_type", "event_data", "ip_address", "user_agent", "created_at", | ||
| }).AddRow( | ||
| id, &connID, "provider.created", `{"name":"google"}`, "127.0.0.1", "curl/7.88", now, |
Comment on lines
+50
to
+56
| Nexus maintains a tamper-evident **audit log** for all control-plane mutations. Every provider create, update, and delete — and every OAuth connection established — writes a record to the `audit_events` table with: | ||
|
|
||
| - The **event type** (`provider.created`, `provider.deleted`, `connection.created`, etc.) | ||
| - **Structured event data** (provider ID, name, workspace ID) | ||
| - The **caller IP address** and **User-Agent** | ||
|
|
||
| This audit log is queryable via the [`GET /audit`](audit-log.md) endpoint and is the foundational building block for compliance, forensic analysis, and detecting unauthorized mutations. |
| @@ -0,0 +1,129 @@ | |||
| # Audit Log Reference | |||
|
|
|||
| The Nexus Broker maintains a tamper-evident **audit log** of every control-plane mutation. Every time a provider is created, updated, or deleted — or an OAuth connection is established — a structured record is written to the `audit_events` table. | |||
Comment on lines
93
to
98
| protected := router.With( | ||
| server.ApiKeyMiddleware(cfg.RequireAPIKey, cfg.APIKeys), | ||
| server.AllowlistMiddleware(cfg.RequireAllowlist, cfg.AllowedCIDRs), | ||
| ) | ||
| protected.Get("/audit", auditHandler.List) | ||
| protected.Route("/providers", func(r chi.Router) { |
| 1. **Fetches** the current live state from `GET /providers`. | ||
| 2. **Diffs** it against your `nexus-providers.yaml` manifest. | ||
| 3. **Prints** a human-readable plan showing creates, updates, and orphaned providers. | ||
| 4. **Applies** the changes only after you confirm with `yes` (or non-interactively in CI). |
- Sort plan diff keys for deterministic output across runs - Track apply failures and os.Exit(1) on partial failure (CI fix) - Normalize provider_id to .String() in all audit log payloads - Wire PatchProfile mock through testify m.Called() (was a dead stub)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the Security-as-Code epic from Issue #11:
Audit Subsystem (Broker)
audit.Service— centralized service that writes to the existingaudit_eventstable with IP validation (net.ParseIP) and User-Agent captureaudit.Loggerinterface — handlers depend on this interface for testability (mock injection in unit tests)ProvidersHandler— logsprovider.created,provider.updated,provider.deletedfor all mutationsCallbackHandler— delegates to the shared audit service, loggingoauth_flow_completed,token_exchange_failed,token_retrieved, etc.client_secretandclient_idwith[REDACTED]to prevent secrets in the audit logGET /auditendpoint — queryable withevent_type,since(RFC3339), andlimitfilters, protected byApiKeyMiddlewareidx_audit_created_atindex for fast descending queriesDeclarative CLI (
nexus-cli)nexus-cli planshows a field-level diff (with secret masking),nexus-cli applyexecutes with confirmation promptGET /providers/{id}(concurrently, bounded worker pool of 5) and computes JSON-level diffsPATCHinstead ofPUTto send only changed fields, preventing accidental nulling of omitted columnsos.Expandwithos.LookupEnvto abort if any${...}placeholders are unresolvedapplyreturns exit code 1 if any operation fails, so CI pipelines correctly detect partial failures--pruneflag — deletions of orphaned providers are opt-in only (defaults tofalse)nexus-providers.yamlwith Google Workspace and GitHub providersDocumentation
docs/guides/security-as-code.md— full guide covering installation, manifest format, commands, and optional CI/CD integrationdocs/reference/audit-log.md— audit event type reference, query examples, response schemadocs/reference/security-model.mdanddocs/services/broker.md— updated to reflect actual event types and unversioned routesTesting
AuditHandler.List(pagination, filtering, parameter validation)MockAuditLoggerCloses #11