Skip to content

Commit

Permalink
Fix vault path-help for selected paths with bad regexps (#18571)
Browse files Browse the repository at this point in the history
* Fix `vault path-help` for selected paths with bad regexps

See the comment being added in `sdk/framework/path.go` for the
explanation of why this change is needed.

* Grammar fix and add changelog

* Also fix hardcoded expectations in a new test

* Add a couple more testcases, and some comments.

* Tweak spelling in comment
  • Loading branch information
maxb authored Jun 30, 2023
1 parent 325233e commit 2f67766
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 7 deletions.
8 changes: 5 additions & 3 deletions api/plugin_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ const (
// path matches that path or not (useful specifically for the paths that
// contain templated fields.)
var sudoPaths = map[string]*regexp.Regexp{
"/auth/token/accessors/": regexp.MustCompile(`^/auth/token/accessors/?$`),
"/auth/token/accessors": regexp.MustCompile(`^/auth/token/accessors/?$`),
"/pki/root": regexp.MustCompile(`^/pki/root$`),
"/pki/root/sign-self-issued": regexp.MustCompile(`^/pki/root/sign-self-issued$`),
"/sys/audit": regexp.MustCompile(`^/sys/audit$`),
Expand All @@ -50,7 +50,7 @@ var sudoPaths = map[string]*regexp.Regexp{
"/sys/config/auditing/request-headers": regexp.MustCompile(`^/sys/config/auditing/request-headers$`),
"/sys/config/auditing/request-headers/{header}": regexp.MustCompile(`^/sys/config/auditing/request-headers/.+$`),
"/sys/config/cors": regexp.MustCompile(`^/sys/config/cors$`),
"/sys/config/ui/headers/": regexp.MustCompile(`^/sys/config/ui/headers/?$`),
"/sys/config/ui/headers": regexp.MustCompile(`^/sys/config/ui/headers/?$`),
"/sys/config/ui/headers/{header}": regexp.MustCompile(`^/sys/config/ui/headers/.+$`),
"/sys/leases": regexp.MustCompile(`^/sys/leases$`),
"/sys/leases/lookup/": regexp.MustCompile(`^/sys/leases/lookup/?$`),
Expand Down Expand Up @@ -249,7 +249,9 @@ func SudoPaths() map[string]*regexp.Regexp {
return sudoPaths
}

// Determine whether the given path requires the sudo capability
// Determine whether the given path requires the sudo capability.
// Note that this uses hardcoded static path information, so will return incorrect results for paths in namespaces,
// or for secret engines mounted at non-default paths.
func IsSudoPath(path string) bool {
// Return early if the path is any of the non-templated sudo paths.
if _, ok := sudoPaths[path]; ok {
Expand Down
19 changes: 19 additions & 0 deletions api/plugin_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ func TestIsSudoPath(t *testing.T) {
path string
expected bool
}{
// Testing: Not a real endpoint
{
"/not/in/sudo/paths/list",
false,
},
// Testing: sys/raw/{path}
{
"/sys/raw/single-node-path",
true,
Expand All @@ -28,26 +30,43 @@ func TestIsSudoPath(t *testing.T) {
"/sys/raw/WEIRD(but_still_valid!)p4Th?🗿笑",
true,
},
// Testing: sys/auth/{path}/tune
{
"/sys/auth/path/in/middle/tune",
true,
},
// Testing: sys/plugins/catalog/{type} and sys/plugins/catalog/{name} (regexes overlap)
{
"/sys/plugins/catalog/some-type",
true,
},
// Testing: Not a real endpoint
{
"/sys/plugins/catalog/some/type/or/name/with/slashes",
false,
},
// Testing: sys/plugins/catalog/{type}/{name}
{
"/sys/plugins/catalog/some-type/some-name",
true,
},
// Testing: Not a real endpoint
{
"/sys/plugins/catalog/some-type/some/name/with/slashes",
false,
},
// Testing: auth/token/accessors (an example of a sudo path that only accepts list operations)
// It is matched as sudo without the trailing slash...
{
"/auth/token/accessors",
true,
},
// ...and also with it.
// (Although at the time of writing, the only caller of IsSudoPath always removes trailing slashes.)
{
"/auth/token/accessors/",
true,
},
}

for _, tc := range testCases {
Expand Down
3 changes: 3 additions & 0 deletions changelog/18571.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
auth/token, sys: Fix path-help being unavailable for some list-only endpoints
```
11 changes: 11 additions & 0 deletions sdk/framework/path.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,17 @@ type Path struct {
// This should be a valid regular expression. Named captures will be
// exposed as fields that should map to a schema in Fields. If a named
// capture is not a field in the Fields map, then it will be ignored.
//
// The pattern will automatically have a ^ prepended and a $ appended before
// use, if these are not already present, so these may be omitted for clarity.
//
// If a ListOperation is being defined, the pattern must end with /? to match
// a trailing slash optionally, as ListOperations are always processed with a
// trailing slash added to the path if not already present. The match must not
// require the presence of a trailing slash, as HelpOperations, even for a
// path which only implements ListOperation, are processed without a trailing
// slash - so failure to make the trailing slash optional will break the
// `vault path-help` command for the path.
Pattern string

// Fields is the mapping of data fields to a schema describing that
Expand Down
4 changes: 2 additions & 2 deletions vault/logical_system_paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ func (b *SystemBackend) configPaths() []*framework.Path {
},

{
Pattern: "config/ui/headers/$",
Pattern: "config/ui/headers/?$",

Operations: map[logical.Operation]framework.OperationHandler{
logical.ListOperation: &framework.PathOperation{
Expand Down Expand Up @@ -1452,7 +1452,7 @@ func (b *SystemBackend) statusPaths() []*framework.Path {
HelpDescription: strings.TrimSpace(sysHelp["ha-status"][1]),
},
{
Pattern: "version-history/$",
Pattern: "version-history/?$",

DisplayAttrs: &framework.DisplayAttributes{
OperationVerb: "version-history",
Expand Down
2 changes: 1 addition & 1 deletion vault/logical_system_pprof.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
func (b *SystemBackend) pprofPaths() []*framework.Path {
return []*framework.Path{
{
Pattern: "pprof/$",
Pattern: "pprof/?$",

DisplayAttrs: &framework.DisplayAttributes{
OperationPrefix: "pprof",
Expand Down
2 changes: 1 addition & 1 deletion vault/token_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ func (ts *TokenStore) paths() []*framework.Path {
},

{
Pattern: "accessors/$",
Pattern: "accessors/?$",

DisplayAttrs: &framework.DisplayAttributes{
OperationPrefix: operationPrefixToken,
Expand Down

0 comments on commit 2f67766

Please sign in to comment.