diff --git a/AGENTS.md b/AGENTS.md index 490a3e63..76c00f35 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -123,7 +123,9 @@ exception: take the fixed version straight away. audits a denial. Diff the rack endpoints in Convox's `sdk/methods.go` between the two releases, and route (`internal/gateway/rbac/http_routes.go`) any that everyday commands call, such as the budget read behind `ps`. The mock rack (`cmd/mock-convox`) has to answer the same way a real rack does, or the CLI E2E tests miss it. Don't - route a new Convox command's own endpoints: they stay refused until someone asks for the command. + route a new Convox command's own endpoints: they stay refused until someone asks for the command. A request the + CLI makes for itself on every command and gets by without on a 404 (like the Console session GET before each + websocket) goes in `isRoutineConvoxCLIRequest` instead, so it isn't audited as a denial. ### 7. DeepSource Static Analysis diff --git a/convox.yml b/convox.yml index 25f4b979..47d96676 100644 --- a/convox.yml +++ b/convox.yml @@ -30,7 +30,7 @@ services: # Pinned to an immutable release tag. scripts/bump-version.sh sets it to the new # version (:vX.Y.Z); the release workflow publishes that tag when the v* git tag is pushed. # :e327add is the v0.1.1 release (identical to :latest when this was pinned). - image: docker.io/docspringcom/rack-gateway:v0.1.22 + image: docker.io/docspringcom/rack-gateway:v0.1.23 command: ./scripts/start-gateway.sh environment: - PORT=8080 @@ -54,7 +54,7 @@ services: admin: # Uses same image as gateway service (kept in sync by scripts/bump-version.sh) - image: docker.io/docspringcom/rack-gateway:v0.1.22 + image: docker.io/docspringcom/rack-gateway:v0.1.23 command: echo "Use this service to run migrations, database admin tasks, etc." environment: - PORT=8080 diff --git a/docs/src/content/docs/security/compliance/audit-trail.mdx b/docs/src/content/docs/security/compliance/audit-trail.mdx index 738d7429..d6391b7d 100644 --- a/docs/src/content/docs/security/compliance/audit-trail.mdx +++ b/docs/src/content/docs/security/compliance/audit-trail.mdx @@ -203,6 +203,13 @@ approval check or the destructive-actions policy, and requests for rack routes t API tokens (`token.validate`) and requests refused by a rate limit (`rate_limit.exceeded`) are recorded the same way. +Three requests that every Convox CLI makes for itself get the same `404` but no audit entry, because they say +nothing about the caller and the CLI works without them. `GET /machines` and `GET /racks` come before each websocket +(build logs, `run`, `exec`, logs) to start a Convox Console session, which a rack doesn't have. +`GET /apps/{app}/configs/cli-env-mask` reads Convox's own env mask before env is printed to a terminal, and the +gateway has already masked secret values. The request log still has them. Any other method or config name is a +denial. + Denials are throttled so a script retrying a refused request can't flood the log. The first denial of an action against a target (the resource, and for a rack request its method and path) for a user (or API token, or for a request without either, its client IP) is written at once, so a denial against one target is never hidden among diff --git a/docs/src/content/docs/security/rbac/permissions.mdx b/docs/src/content/docs/security/rbac/permissions.mdx index 8c9eaeb5..9eb4aa51 100644 --- a/docs/src/content/docs/security/rbac/permissions.mdx +++ b/docs/src/content/docs/security/rbac/permissions.mdx @@ -56,6 +56,8 @@ The gateway forwards only the rack API routes it has a permission for (see [Rack API Routes](#rack-api-routes)). Requests to any other route are refused with `404` and logged as denied, whatever the caller's role, so Convox's certificates, registries, resources, instance keyroll and terminate, and canceling an app update aren't available through the gateway (the `rack-gateway` CLI has no commands for them). +Three requests the Convox CLI makes for itself are refused without a denial entry (see +[Denied Requests](/security/compliance/audit-trail/#denied-requests)). ### Gateway Resources diff --git a/internal/gateway/AGENTS.md b/internal/gateway/AGENTS.md index 915874cb..0dd45ac0 100644 --- a/internal/gateway/AGENTS.md +++ b/internal/gateway/AGENTS.md @@ -153,10 +153,11 @@ Structured logging: roles' grants) in one migration: the previous release keeps serving requests between `migrate` and the promote, and it refuses every request it can't record, the promote included. Add the new function alongside the old one, and drop the old one in a later release -- Denials (gateway `Authorize`, proxy RBAC/approval/destructive refusals, unknown rack routes) go through - `RecordDenial`: throttled per principal + action + target (resource, method and path) per minute in memory (one - gateway per rack). Past 1,000 runs, each principal's denials without a run share one, so memory stays bounded. - The request is refused either way; a denial that can't be written is logged and sent to Sentry +- Denials (gateway `Authorize`, proxy RBAC/approval/destructive refusals, unknown rack routes other than the Convox + CLI's own requests in `isRoutineConvoxCLIRequest`) go through `RecordDenial`: throttled per principal + action + + target (resource, method and path) per minute in memory (one gateway per rack). Past 1,000 runs, each principal's + denials without a run share one, so memory stays bounded. The request is refused either way; a denial that can't + be written is logged and sent to Sentry Key files: diff --git a/internal/gateway/proxy/convox_cli_requests.go b/internal/gateway/proxy/convox_cli_requests.go new file mode 100644 index 00000000..371a0e35 --- /dev/null +++ b/internal/gateway/proxy/convox_cli_requests.go @@ -0,0 +1,32 @@ +package proxy + +import ( + "net/http" + "strings" +) + +// envMaskConfigName is the app config Convox's CLI keeps its env mask in (`convox env mask set`). +const envMaskConfigName = "cli-env-mask" + +// isRoutineConvoxCLIRequest reports whether a request to a rack route the gateway doesn't forward is one every +// Convox CLI makes for itself and gets by fine without. The gateway still answers it with 404, but doesn't audit it +// as a denial: it says nothing about the caller, and every deploy, run and log stream would otherwise add one. +// +// - GET /machines and GET /racks: before each websocket (build logs, run, exec, logs), the Convox SDK sends a GET +// that starts a Convox Console session, and ignores the answer. A rack has neither route. +// - GET /apps/{app}/configs/cli-env-mask: `convox env` and `convox releases info` read Convox's env mask before +// printing to a terminal, and print the env as it came on a 404. The gateway has already masked secret values. +// +// Any other method or config name (a `convox env mask set`, a read of another config) is still a denial. path is +// the canonical rack path, already checked by isSafeRackPath. +func isRoutineConvoxCLIRequest(method, path string) bool { + if method != http.MethodGet { + return false + } + if path == "/machines" || path == "/racks" { + return true + } + segments := strings.Split(path, "/") + return len(segments) == 5 && segments[0] == "" && segments[1] == "apps" && segments[2] != "" && + segments[3] == "configs" && segments[4] == envMaskConfigName +} diff --git a/internal/gateway/proxy/convox_cli_requests_test.go b/internal/gateway/proxy/convox_cli_requests_test.go new file mode 100644 index 00000000..b6269d24 --- /dev/null +++ b/internal/gateway/proxy/convox_cli_requests_test.go @@ -0,0 +1,92 @@ +package proxy + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/DocSpring/rack-gateway/internal/gateway/audit" +) + +func TestIsRoutineConvoxCLIRequest(t *testing.T) { + routine := []string{"/machines", "/racks", "/apps/my-app/configs/cli-env-mask"} + for _, path := range routine { + require.Truef(t, isRoutineConvoxCLIRequest(http.MethodGet, path), "GET %s", path) + } + + notRoutine := []struct{ method, path string }{ + {http.MethodPut, "/apps/my-app/configs/cli-env-mask"}, + {http.MethodPost, "/machines"}, + {"SOCKET", "/machines"}, + {http.MethodGet, "/machines/m1"}, + {http.MethodGet, "/racks/staging"}, + {http.MethodGet, "/apps/my-app/configs/other"}, + {http.MethodGet, "/apps/my-app/configs"}, + {http.MethodGet, "/apps//configs/cli-env-mask"}, + {http.MethodGet, "/apps/my-app/configs/cli-env-mask/x"}, + {http.MethodGet, "/x/apps/my-app/configs/cli-env-mask"}, + } + for _, tc := range notRoutine { + require.Falsef(t, isRoutineConvoxCLIRequest(tc.method, tc.path), "%s %s", tc.method, tc.path) + } +} + +// TestRoutineConvoxCLIRequestsAreNotAuditedAsDenials: the requests every Convox CLI makes for itself get the same +// 404 as any route the gateway doesn't forward, but leave no denial in the audit log, repeat counts included. +func TestRoutineConvoxCLIRequestsAreNotAuditedAsDenials(t *testing.T) { + h, database, rack, cleanup := newAttemptProxy(t) + defer cleanup() + + for _, path := range []string{ + "/machines", "/racks", "/apps/my-app/configs/cli-env-mask", "/convox/machines", "/machines", + } { + rr := proxyAs(t, h, database, http.MethodGet, path) + require.Equalf(t, http.StatusNotFound, rr.Code, "GET %s", path) + } + require.Zero(t, rack.calls.Load()) + + h.auditLogger.FlushDenials() + require.Empty(t, attemptEntries(t, database)) +} + +// TestNearRoutineRequestsAreStillAuditedAsDenials: a request that only resembles one of the Convox CLI's own (another +// method, another config, a websocket) is refused and audited like any other route the gateway doesn't forward. +func TestNearRoutineRequestsAreStillAuditedAsDenials(t *testing.T) { + cases := map[string]struct { + method, path string + websocket bool + }{ + "env mask write": {method: http.MethodPut, path: "/apps/my-app/configs/cli-env-mask"}, + "another config": {method: http.MethodGet, path: "/apps/my-app/configs/secrets"}, + "a machine": {method: http.MethodGet, path: "/machines/m1"}, + "websocket": {method: http.MethodGet, path: "/machines", websocket: true}, + "dot segment app": {method: http.MethodGet, path: "/apps/../configs/cli-env-mask"}, + "query in app (%3F)": {method: http.MethodGet, path: "/apps/a?b/configs/cli-env-mask"}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + h, database, rack, cleanup := newAttemptProxy(t) + defer cleanup() + + req := httptest.NewRequest(tc.method, "/", nil) + req.URL.Path = tc.path + req = req.WithContext(requestAs(t, database, attemptUserEmail).Context()) + if tc.websocket { + req.Header.Set("Connection", "Upgrade") + req.Header.Set("Upgrade", "websocket") + } + rr := httptest.NewRecorder() + h.ProxyToRack(rr, req) + require.Equal(t, http.StatusNotFound, rr.Code) + require.Zero(t, rack.calls.Load()) + + entries := attemptEntries(t, database) + require.Len(t, entries, 1) + require.Equal(t, audit.StatusDenied, entries[0].Status) + require.Equal(t, http.StatusNotFound, entries[0].HTTPStatus) + require.Contains(t, entries[0].Details, `"reason":"the gateway doesn't forward this rack route"`) + }) + } +} diff --git a/internal/gateway/proxy/handler.go b/internal/gateway/proxy/handler.go index 8332fb9a..ea16d748 100644 --- a/internal/gateway/proxy/handler.go +++ b/internal/gateway/proxy/handler.go @@ -279,7 +279,8 @@ func (h *Handler) prepareProxyRequest( // resolveRackRoute rewrites r to its canonical rack path and finds the RBAC resource and action for it. A path // that isn't an allowed rack route is answered with 404 and audited as a denial, so probing the rack for routes the -// gateway doesn't forward leaves a record. +// gateway doesn't forward leaves a record. The requests every Convox CLI makes for itself +// (isRoutineConvoxCLIRequest) aren't audited. func (h *Handler) resolveRackRoute( w http.ResponseWriter, r *http.Request, @@ -289,17 +290,18 @@ func (h *Handler) resolveRackRoute( ) (*http.Request, rbac.Resource, rbac.Action, error) { r, ok := canonicalRackRequest(r) method := h.determineMethod(r) + safe := ok && isSafeRackPath(r.URL.Path) var resource rbac.Resource var action rbac.Action - if ok && isSafeRackPath(r.URL.Path) { + if safe { resource, action, ok = rbac.MatchRackRoute(method, r.URL.Path) - } else { - ok = false } - if !ok { + if !safe || !ok { err := fmt.Errorf("unknown route: %s %s", method, r.URL.Path) h.auditLogger.LogRequest(r, authUser.Email, rackName, "deny", http.StatusNotFound, time.Since(start), err) - h.recordDenial(r, authUser, http.StatusNotFound, "the gateway doesn't forward this rack route") + if !safe || !isRoutineConvoxCLIRequest(method, r.URL.Path) { + h.recordDenial(r, authUser, http.StatusNotFound, "the gateway doesn't forward this rack route") + } http.NotFound(w, r) return r, 0, 0, errors.New("unknown route") } diff --git a/web/package.json b/web/package.json index ad679f1e..01882145 100644 --- a/web/package.json +++ b/web/package.json @@ -1,6 +1,6 @@ { "name": "web", - "version": "0.1.22", + "version": "0.1.23", "private": true, "type": "module", "scripts": {