admin: normalize request path in remote admin access-control check (defense-in-depth)

[AmariahAK]

(References #7909)

The remote admin API's access-control function adminPathAllowed compared
raw request paths against allowed paths without normalization. A request
like /pki/ca/prod/../../../../load would pass the prefix check for an
identity scoped to /pki/ca/prod, even though the path resolves to
/load.

While not exploitable today (the router redirects .. paths before
dispatch), the access-control layer depends on the router for safety.
This is defense-in-depth: normalize both the request path and allowed
path with path.Clean (purely lexical, no filesystem access) before
the boundary check. This preserves the existing segment-boundary
semantics (e.g., /pki must not match /pkisecret).

Added regression tests for: dot-dot traversal, encoded traversal,
sibling traversal, collapsed slashes, and trailing-slash requests.
All five fail without the fix and pass with it.

Assistance Disclosure

Atlarix agent (DeepSeek) generated the test cases and the two-line
path.Clean implementation. I authored the fix concept/approach,
reviewed every line, and verified correctness including running the
full test suite and confirming the tests fail without the fix and
pass with it.

[CLAassistant]

All committers have signed the CLA.

[iabdullah215]

Hi @AmariahAK: Thanks for putting this together. I'm the author of the referenced issue #7909; I'd also prepared a fix for this locally (same path.Clean normalization on both the request path and allowed path, plus regression tests for dot-dot/sibling/collapsed-slash/trailing-slash cases) and was about to open a PR.
Happy to see it addressed either way, I'll review the diff here and leave any notes. A couple of things worth double-checking that I covered in my version: that the segment-boundary semantics are preserved (e.g. /pki must still not match /pkisecret), and behavior on an exact match with a trailing slash.

[AmariahAK]

@iabdullah215 thanks for the review, ill wait for the full list of things you find so that i can fix them all in one clean sweep
But let me take a look into what youve asked for

[iabdullah215]

Took a proper look, the core path.Clean on reqPath is the right fix. Two things worth folding in:

1. path.Clean("") returns ".", not "". Cleaning allowedPath unconditionally means an empty allowed path (Paths: [""]) becomes ".", which no longer satisfies the allowedPath == "" || allowedPath == "/" guard and falls through to deny. Upstream today treats an empty allowedPath as allow-all (HasPrefix(reqPath, "") is always true), so this silently changes that behavior. Safest to short-circuit the empty case before cleaning:

 reqPath = path.Clean(reqPath)
 if allowedPath == "" {
     return true
 }
 allowedPath = path.Clean(allowedPath)
 if allowedPath == "/" {
     return true
 }
 return reqPath == allowedPath || strings.HasPrefix(reqPath, allowedPath+"/")

2. Minor: once allowedPath is cleaned it can never have a trailing slash, so the strings.HasSuffix(allowedPath, "/") branch below becomes dead code, the reqPath == allowedPath || HasPrefix(reqPath, allowedPath+"/") form covers it and reads more clearly.

(For what it's worth, the %2e%2e test is correct as-is, the request harness decodes it to .. before path.Clean runs.)

[AmariahAK]

@iabdullah215 thanks for this, let me get to working on them

[AmariahAK]

@iabdullah215 could you take a look, should be good to go now

[iabdullah215]

Looks good, the empty allowedPath short-circuit before path.Clean and the simplified final check both address what I raised, and the test coverage is solid. LGTM from my side. Thanks for turning this around quickly.

[iabdullah215]

Glad it worked out. Since the empty-path fix came from my local version, would you mind adding a co-author trailer to the commit? Co-authored-by: iabdullah215 <muhammadabdullah8040@gmail.com>, that way we both show as contributors. No worries if you'd rather not.

[AmariahAK]

@iabdullah215 done and thanks for the review
also your in the latest commit as well