From b1b11330a6a2773b1241bd49b4c7a75475b6bbc6 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sun, 30 Aug 2026 14:45:04 +1000 Subject: [PATCH] Add per-role HTTP method scoping to minted tokens (#2) ## Why Every token this engine mints is as powerful as the apps it can reach, so a read-only integration can still write to the *arr. arrproxy now accepts a method scope at mint time, and the engine has no way to ask for one. ## How - Add an optional `methods` role field, uppercase-normalized and de-duplicated. - Reject a method outside GET/HEAD/POST/PUT/PATCH/DELETE/OPTIONS at role write. - Forward the role's scope on the arrproxy mint request and echo it in the creds response. - Omit the field when a role has no scope, so unscoped roles behave exactly as before. - Cover normalization, rejection, pass-through and the unscoped case. Requires arrproxy >= v0.5.0 deployed. Reviewed-on: https://git.unkin.net/unkin/vault-plugin-secrets-arrstack/pulls/2 Co-authored-by: unkin-agent Co-committed-by: unkin-agent --- README.md | 25 ++++++++ backend_test.go | 33 +++++++++- client.go | 7 ++- client_test.go | 33 ++++++++++ path_credentials.go | 2 + path_credentials_test.go | 57 +++++++++++++++++ path_roles.go | 47 ++++++++++++++ path_roles_test.go | 129 +++++++++++++++++++++++++++++++++++++++ 8 files changed, 329 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 569093d..8bf1766 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,8 @@ bearer-gated **admin API** (`/api/admin/tokens`) instead, so Terraform-driven Each minted token is: - **scoped to a subset of apps** (`apps`: any of `sonarr`, `radarr`, `prowlarr`) +- **optionally scoped to HTTP methods** (`methods`, e.g. `GET,HEAD` for a + read-only consumer; empty means unrestricted) - **subject-namespaced** as `vault:arrstack:` (arrproxy requires the prefix) - **bound to a Vault lease** — revoking the lease disables the token in arrproxy @@ -49,6 +51,7 @@ vault write arrstack/config \ ca_cert=@traefik-external-ca.pem # optional # 3. Define a role: which apps, what TTLs +# (add methods="GET,HEAD" to pin the token to those HTTP methods) vault write arrstack/roles/media \ apps="sonarr,radarr" \ ttl=1h \ @@ -61,6 +64,7 @@ vault read arrstack/creds/media # lease_id arrstack/creds/media/AbC... # lease_duration 1h # apps [radarr sonarr] +# methods [] # id tok-... # subject vault:arrstack:media # token arr_... @@ -108,9 +112,30 @@ vault write arrstack/roles/all apps="sonarr,radarr,prowlarr" ttl=1h max_t | Field | Type | Description | | --------- | -------- | ------------------------------------------------------------------ | | `apps` | list | Non-empty subset of `sonarr`/`radarr`/`prowlarr` | +| `methods` | list | Optional HTTP methods the token is limited to; empty = unrestricted | | `ttl` | duration | Default lease TTL | | `max_ttl` | duration | Maximum lease TTL | +## Method scoping + +A role may pin its tokens to a set of HTTP methods, so a read-only integration +cannot write to the *arr even on an app it is allowed to reach. Entries are +uppercase-normalized and must name one of `GET`, `HEAD`, `POST`, `PUT`, +`PATCH`, `DELETE`, `OPTIONS` — an unknown method is rejected at role write, not +at mint. The scope is sent to arrproxy at mint time and enforced there: a +request outside it is refused with `405` before any upstream *arr is reached. + +Leaving `methods` unset (the default) leaves the token unrestricted, exactly as +before this field existed. + +```sh +# A token that can read Sonarr but never change it. +vault write arrstack/roles/sonarr-ro apps="sonarr" methods="GET,HEAD" ttl=1h max_ttl=24h + +# Drop the scope again on an existing role. +vault write arrstack/roles/sonarr-ro methods="" +``` + ## TTL and renewal At mint time the effective initial lease TTL is sent to arrproxy as diff --git a/backend_test.go b/backend_test.go index 029cf75..1a196db 100644 --- a/backend_test.go +++ b/backend_test.go @@ -3,6 +3,7 @@ package arrstack import ( "context" "encoding/json" + "io" "net/http" "net/http/httptest" "strconv" @@ -33,6 +34,7 @@ func getTestBackend(t *testing.T) (*arrstackBackend, logical.Storage) { type mockToken struct { Subject string Apps []string + Methods []string Label string ExpiresAt *time.Time Disabled bool @@ -49,9 +51,13 @@ type mockArrproxy struct { counter int adminToken string apps map[string]bool + methods map[string]bool - mintErr bool + mintErr bool + // lastRequest is the decoded mint payload; lastBody is the raw JSON, so a + // test can assert a field was omitted rather than sent empty. lastRequest mintTokenRequest + lastBody []byte } func newMockArrproxy(t *testing.T) *mockArrproxy { @@ -60,6 +66,10 @@ func newMockArrproxy(t *testing.T) *mockArrproxy { tokens: make(map[string]*mockToken), adminToken: "arrproxy-admin-secret", apps: map[string]bool{"sonarr": true, "radarr": true, "prowlarr": true}, + methods: map[string]bool{ + "GET": true, "HEAD": true, "POST": true, "PUT": true, + "PATCH": true, "DELETE": true, "OPTIONS": true, + }, } mux := http.NewServeMux() @@ -108,12 +118,18 @@ func (m *mockArrproxy) handleMint(w http.ResponseWriter, r *http.Request) { return } + body, err := io.ReadAll(r.Body) + if err != nil { + http.Error(w, "bad request", http.StatusBadRequest) + return + } var req mintTokenRequest - if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + if err := json.Unmarshal(body, &req); err != nil { http.Error(w, "bad request", http.StatusBadRequest) return } m.lastRequest = req + m.lastBody = body if !strings.HasPrefix(req.Subject, "vault:arrstack:") || strings.TrimSpace(strings.TrimPrefix(req.Subject, "vault:arrstack:")) == "" { http.Error(w, "subject must be namespaced with vault:arrstack:", http.StatusBadRequest) @@ -129,6 +145,13 @@ func (m *mockArrproxy) handleMint(w http.ResponseWriter, r *http.Request) { return } } + // An absent or empty methods list is unrestricted. + for _, meth := range req.Methods { + if !m.methods[meth] { + http.Error(w, "methods must name known HTTP methods", http.StatusBadRequest) + return + } + } if req.TTLSeconds < 0 { http.Error(w, "ttl_seconds must not be negative", http.StatusBadRequest) return @@ -141,9 +164,14 @@ func (m *mockArrproxy) handleMint(w http.ResponseWriter, r *http.Request) { exp := time.Now().UTC().Add(time.Duration(req.TTLSeconds) * time.Second) expiresAt = &exp } + methods := req.Methods + if methods == nil { + methods = []string{} + } m.tokens[id] = &mockToken{ Subject: req.Subject, Apps: req.Apps, + Methods: methods, Label: req.Label, ExpiresAt: expiresAt, } @@ -151,6 +179,7 @@ func (m *mockArrproxy) handleMint(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusCreated, map[string]interface{}{ "id": id, "token": "arr_" + id + "_plaintext", + "methods": methods, "expires_at": expiresAt, }) } diff --git a/client.go b/client.go index ca6e4e6..1c6d18f 100644 --- a/client.go +++ b/client.go @@ -63,8 +63,11 @@ func newClient(config *arrstackConfig) (*arrproxyClient, error) { // mintTokenRequest is the payload for POST /api/admin/tokens. The subject MUST // carry the "vault:arrstack:" prefix or arrproxy rejects the request. type mintTokenRequest struct { - Subject string `json:"subject"` - Apps []string `json:"apps"` + Subject string `json:"subject"` + Apps []string `json:"apps"` + // Methods limits the token to those HTTP methods. Omitted when empty so + // arrproxy versions predating method scoping see the request unchanged. + Methods []string `json:"methods,omitempty"` Label string `json:"label"` TTLSeconds int64 `json:"ttl_seconds"` } diff --git a/client_test.go b/client_test.go index fb57715..0109ac1 100644 --- a/client_test.go +++ b/client_test.go @@ -2,6 +2,7 @@ package arrstack import ( "context" + "strings" "testing" ) @@ -35,6 +36,38 @@ func TestClient_MintToken(t *testing.T) { } } +func TestClient_MintToken_ForwardsMethods(t *testing.T) { + m := newMockArrproxy(t) + client, _ := newClient(&arrstackConfig{BaseURL: m.server.URL, AdminToken: m.adminToken}) + + if _, err := client.MintToken(context.Background(), mintTokenRequest{ + Subject: "vault:arrstack:readonly", + Apps: []string{"sonarr"}, + Methods: []string{"GET", "HEAD"}, + TTLSeconds: 60, + }); err != nil { + t.Fatalf("MintToken: %v", err) + } + if strings.Join(m.lastRequest.Methods, ",") != "GET,HEAD" { + t.Fatalf("expected methods forwarded, got %v", m.lastRequest.Methods) + } +} + +func TestClient_MintToken_RejectsUnknownMethod(t *testing.T) { + m := newMockArrproxy(t) + client, _ := newClient(&arrstackConfig{BaseURL: m.server.URL, AdminToken: m.adminToken}) + + _, err := client.MintToken(context.Background(), mintTokenRequest{ + Subject: "vault:arrstack:readonly", + Apps: []string{"sonarr"}, + Methods: []string{"FETCH"}, + TTLSeconds: 60, + }) + if err == nil { + t.Fatal("expected arrproxy to reject an unknown HTTP method") + } +} + func TestClient_MintToken_RejectsBadSubject(t *testing.T) { m := newMockArrproxy(t) client, _ := newClient(&arrstackConfig{BaseURL: m.server.URL, AdminToken: m.adminToken}) diff --git a/path_credentials.go b/path_credentials.go index 1bb8bab..16cba7e 100644 --- a/path_credentials.go +++ b/path_credentials.go @@ -75,6 +75,7 @@ func (b *arrstackBackend) mintToken(ctx context.Context, req *logical.Request, r minted, err := client.MintToken(ctx, mintTokenRequest{ Subject: subject, Apps: role.Apps, + Methods: role.Methods, Label: label, TTLSeconds: int64(ttl.Seconds()), }) @@ -98,6 +99,7 @@ func (b *arrstackBackend) mintToken(ctx context.Context, req *logical.Request, r "token": minted.Token, "id": minted.ID, "apps": role.Apps, + "methods": role.Methods, "subject": subject, "expires_at": expiresAt.Format(time.RFC3339), } diff --git a/path_credentials_test.go b/path_credentials_test.go index afa5b16..d624ab2 100644 --- a/path_credentials_test.go +++ b/path_credentials_test.go @@ -93,6 +93,63 @@ func TestCredentials_MintAndRevoke(t *testing.T) { } } +func TestCredentials_MethodsForwardedAndEchoed(t *testing.T) { + b, s := getTestBackend(t) + m := newMockArrproxy(t) + writeTestConfig(t, b, s, m.server.URL, m.adminToken) + createRole(t, b, s, "readonly", map[string]interface{}{ + "apps": "sonarr", + "methods": "head,get", + "ttl": "1h", + }) + + resp, err := b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.ReadOperation, + Path: "creds/readonly", + Storage: s, + }) + if err != nil || resp == nil { + t.Fatalf("mint creds: err=%v resp=%v", err, resp) + } + + // The role's normalized method scope reached arrproxy. + if strings.Join(m.lastRequest.Methods, ",") != "GET,HEAD" { + t.Fatalf("expected methods GET,HEAD forwarded, got %v", m.lastRequest.Methods) + } + // And is echoed back to the caller alongside apps/subject. + if strings.Join(resp.Data["methods"].([]string), ",") != "GET,HEAD" { + t.Fatalf("expected methods echoed in creds response, got %v", resp.Data["methods"]) + } +} + +func TestCredentials_NoMethodsOmitsTheField(t *testing.T) { + b, s := getTestBackend(t) + m := newMockArrproxy(t) + writeTestConfig(t, b, s, m.server.URL, m.adminToken) + createRole(t, b, s, "media", map[string]interface{}{"apps": "sonarr", "ttl": "1h"}) + + resp, err := b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.ReadOperation, + Path: "creds/media", + Storage: s, + }) + if err != nil || resp == nil { + t.Fatalf("mint creds: err=%v resp=%v", err, resp) + } + + // An unscoped role sends no methods key at all, so an arrproxy predating + // method scoping sees the request exactly as before. + if strings.Contains(string(m.lastBody), "methods") { + t.Fatalf("expected methods omitted from the mint body, got %s", m.lastBody) + } + if len(m.lastRequest.Methods) != 0 { + t.Fatalf("expected no methods forwarded, got %v", m.lastRequest.Methods) + } + if methods, ok := resp.Data["methods"].([]string); ok && len(methods) != 0 { + t.Fatalf("expected an empty method scope in the response, got %v", methods) + } +} + func TestCredentials_UnknownRole(t *testing.T) { b, s := getTestBackend(t) m := newMockArrproxy(t) diff --git a/path_roles.go b/path_roles.go index 22cf559..5ecfbd6 100644 --- a/path_roles.go +++ b/path_roles.go @@ -3,7 +3,9 @@ package arrstack import ( "context" "fmt" + "net/http" "sort" + "strings" "time" "github.com/hashicorp/vault/sdk/framework" @@ -20,10 +22,21 @@ var knownApps = map[string]bool{ "prowlarr": true, } +// knownMethods mirrors arrproxy's accepted HTTP method scope. Validating here +// rejects a typo at role write instead of at mint. +var knownMethods = map[string]bool{ + http.MethodGet: true, http.MethodHead: true, http.MethodPost: true, + http.MethodPut: true, http.MethodPatch: true, http.MethodDelete: true, + http.MethodOptions: true, +} + // arrstackRole constrains the machine tokens minted from the creds/ path. type arrstackRole struct { // Apps is the subset of sonarr/radarr/prowlarr a minted token may reach. Apps []string `json:"apps"` + // Methods, when non-empty, limits a minted token to those HTTP methods. + // Empty means unrestricted. + Methods []string `json:"methods,omitempty"` // TTL is the default lease duration for tokens issued from this role. TTL time.Duration `json:"ttl"` // MaxTTL is the maximum lease duration for tokens issued from this role. @@ -33,6 +46,7 @@ type arrstackRole struct { func (r *arrstackRole) toResponseData() map[string]interface{} { return map[string]interface{}{ "apps": r.Apps, + "methods": r.Methods, "ttl": int64(r.TTL.Seconds()), "max_ttl": int64(r.MaxTTL.Seconds()), } @@ -59,6 +73,28 @@ func validateApps(requested []string) ([]string, error) { return out, nil } +// validateMethods uppercase-normalizes, de-duplicates and sorts a requested +// method scope. An empty request yields a nil scope, meaning unrestricted. +func validateMethods(requested []string) ([]string, error) { + if len(requested) == 0 { + return nil, nil + } + seen := map[string]bool{} + out := make([]string, 0, len(requested)) + for _, m := range requested { + m = strings.ToUpper(strings.TrimSpace(m)) + if !knownMethods[m] { + return nil, fmt.Errorf("unknown method %q: methods must be a subset of GET/HEAD/POST/PUT/PATCH/DELETE/OPTIONS", m) + } + if !seen[m] { + seen[m] = true + out = append(out, m) + } + } + sort.Strings(out) + return out, nil +} + func pathRole(b *arrstackBackend) *framework.Path { return &framework.Path{ Pattern: "roles/" + framework.GenericNameRegex("name"), @@ -77,6 +113,10 @@ func pathRole(b *arrstackBackend) *framework.Path { Description: "Comma-separated subset of sonarr/radarr/prowlarr a minted token may reach.", Required: true, }, + "methods": { + Type: framework.TypeCommaStringSlice, + Description: "Optional comma-separated HTTP methods a minted token is limited to (GET/HEAD/POST/PUT/PATCH/DELETE/OPTIONS). Empty means unrestricted.", + }, "ttl": { Type: framework.TypeDurationSecond, Description: "Default lease TTL for tokens minted from this role.", @@ -171,6 +211,13 @@ func (b *arrstackBackend) pathRoleWrite(ctx context.Context, req *logical.Reques } role.Apps = apps } + if v, ok := data.GetOk("methods"); ok { + methods, err := validateMethods(v.([]string)) + if err != nil { + return logical.ErrorResponse(err.Error()), nil + } + role.Methods = methods + } if v, ok := data.GetOk("ttl"); ok { role.TTL = time.Duration(v.(int)) * time.Second } diff --git a/path_roles_test.go b/path_roles_test.go index defa4b1..1b2b596 100644 --- a/path_roles_test.go +++ b/path_roles_test.go @@ -2,6 +2,7 @@ package arrstack import ( "context" + "strings" "testing" "github.com/hashicorp/vault/sdk/logical" @@ -148,6 +149,134 @@ func TestRole_TTLGreaterThanMaxTTLRejected(t *testing.T) { } } +func TestRole_MethodsStoredNormalized(t *testing.T) { + b, s := getTestBackend(t) + ctx := context.Background() + + resp, err := b.HandleRequest(ctx, &logical.Request{ + Operation: logical.CreateOperation, + Path: "roles/readonly", + Storage: s, + Data: map[string]interface{}{ + "apps": "sonarr", + "methods": "get, head ,GET", + }, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("write role: err=%v resp=%v", err, resp) + } + + resp, err = b.HandleRequest(ctx, &logical.Request{ + Operation: logical.ReadOperation, + Path: "roles/readonly", + Storage: s, + }) + if err != nil || resp == nil { + t.Fatalf("read role: err=%v resp=%v", err, resp) + } + methods := resp.Data["methods"].([]string) + // Uppercased, de-duplicated and sorted. + if strings.Join(methods, ",") != "GET,HEAD" { + t.Fatalf("unexpected methods: %v", methods) + } +} + +func TestRole_MethodsDefaultEmpty(t *testing.T) { + b, s := getTestBackend(t) + ctx := context.Background() + + resp, err := b.HandleRequest(ctx, &logical.Request{ + Operation: logical.CreateOperation, + Path: "roles/unrestricted", + Storage: s, + Data: map[string]interface{}{"apps": "sonarr"}, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("write role: err=%v resp=%v", err, resp) + } + + role, _ := b.getRole(ctx, s, "unrestricted") + if len(role.Methods) != 0 { + t.Fatalf("expected no method scope by default, got %v", role.Methods) + } +} + +func TestRole_UnknownMethodRejected(t *testing.T) { + b, s := getTestBackend(t) + + resp, err := b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.CreateOperation, + Path: "roles/bad", + Storage: s, + Data: map[string]interface{}{"apps": "sonarr", "methods": "GET,FETCH"}, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if resp == nil || !resp.IsError() { + t.Fatal("expected an error for a method outside the known HTTP set") + } + role, _ := b.getRole(context.Background(), s, "bad") + if role != nil { + t.Fatal("expected the role not to be stored when methods are invalid") + } +} + +func TestRole_MethodsClearedOnUpdate(t *testing.T) { + b, s := getTestBackend(t) + ctx := context.Background() + + createRole(t, b, s, "media", map[string]interface{}{"apps": "sonarr", "methods": "GET"}) + + resp, err := b.HandleRequest(ctx, &logical.Request{ + Operation: logical.UpdateOperation, + Path: "roles/media", + Storage: s, + Data: map[string]interface{}{"methods": ""}, + }) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("update role: err=%v resp=%v", err, resp) + } + role, _ := b.getRole(ctx, s, "media") + if len(role.Methods) != 0 { + t.Fatalf("expected methods cleared, got %v", role.Methods) + } + if len(role.Apps) != 1 { + t.Fatalf("expected apps preserved across the update, got %v", role.Apps) + } +} + +func TestValidateMethods(t *testing.T) { + cases := []struct { + name string + in []string + want []string + wantErr bool + }{ + {"empty is unrestricted", nil, nil, false}, + {"lowercase normalized", []string{"get"}, []string{"GET"}, false}, + {"trimmed dedup and sort", []string{" head ", "GET", "head"}, []string{"GET", "HEAD"}, false}, + {"all known", []string{"OPTIONS", "DELETE", "PATCH", "PUT", "POST", "HEAD", "GET"}, + []string{"DELETE", "GET", "HEAD", "OPTIONS", "PATCH", "POST", "PUT"}, false}, + {"unknown", []string{"GET", "FETCH"}, nil, true}, + {"empty entry", []string{""}, nil, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := validateMethods(tc.in) + if (err != nil) != tc.wantErr { + t.Fatalf("validateMethods err=%v wantErr=%v", err, tc.wantErr) + } + if tc.wantErr { + return + } + if strings.Join(got, ",") != strings.Join(tc.want, ",") { + t.Fatalf("got %v want %v", got, tc.want) + } + }) + } +} + func TestValidateApps(t *testing.T) { cases := []struct { name string