Skip to content

Commit 9f33b47

Browse files
authored
[security] Reserve spike/system/* from substring policy matches (#301)
* tasks. Signed-off-by: Volkan Özçelik <volkan.ozcelik@broadcom.com> * fix(nexus): reserve spike/system/* from substring policy matches This commit has two independent parts. They are unrelated to each other and are together only because the integration suite had been sitting uncommitted in the working tree since 2026-07-18; folding it in now avoids stranding it further behind an unrelated branch. Part 1 is the substantive change. Part 2 is previously written work, committed here unmodified apart from being verified to still build, vet, and lint under its build tag. ================================================================ Part 1: reserved system namespaces (Spec: policy-pattern-anchoring) ================================================================ Policy patterns are regular expressions matched with MatchString, so an unanchored pattern is a substring test. That is intended behavior for ordinary paths and stays unchanged. It was not acceptable for the three paths through which SPIKE authorizes its own privileged operations. Policy management is gated by CheckPolicyAccess against the literal path spike/system/acl. A policy whose PathPattern was "spike", "system", or "acl" therefore matched that gate by substring and authorized the workload to create and modify any policy, including one granting itself super on every path. PathPattern "spike" is plausible for an operator whose own secrets live under a spike/ namespace. The same reached spike/system/secret and spike/system/cipher/exec. Nothing reserved those namespaces. A policy may now reach a reserved path only when it describes that path rather than merely containing it: its full-match form, ^(?:pattern)$, must still match. So ^spike/system/acl$, spike/system/acl, ^spike/system/.*$ and .* all qualify, while acl, system and spike do not. The SPIFFE ID pattern must be anchored or an unambiguous catch-all, so a delegation written for spiffe://example.org/admin cannot be claimed by spiffe://example.org/admin-attacker. Enforcement sits at two points. UpsertPolicy rejects a violating policy so the operator learns at authoring time, and CheckPolicyAccess declines to honor one independently, covering policies stored before the rule existed. The second point is load-bearing rather than redundant: the SQLite backend recompiles both patterns from the stored strings on every access check, so a guard placed only at creation would hold for the in-memory backend and do nothing in production. A purely syntactic "must start with ^ and end with $" rule was implemented first and rejected. .* and ^.*$ are the same regular expression, so accepting one and refusing the other polices spelling without changing what is granted, and it broke deliberate wildcard policies already under test. Documentation is corrected throughout. The reference page carried a "Path Pattern Examples" block of ^-only prefixes, one annotated "Only the specific creds resource" for a pattern that also matches secrets/database/credsXYZ, and a "Common Errors" section that offered unanchored patterns as the remediation. CLAUDE.md framed the only correctness axis as regex-versus-glob and marked two unanchored patterns correct. Also fixes a shipped glob in the federation example and the non-existent YAML keys in sample-policy.yaml, which could never have loaded. The rendered site is rebuilt. That picks up the Recipes section and the multi-tenancy page, neither of which had ever been rendered, and drops ten orphaned pages under docs/getting-started/ that had no source in docs-src; one of them was still serving the superseded policy guidance at a live URL. This accounts for most of the file count here. Reported by kanywst. The substring behavior they reported is documented and intended; the escalation path underneath it was not, and was found while assessing the report. ================================================================ Part 2: live Pilot integration suite, Slice B (Spec: integration-tests) ================================================================ Written 2026-07-18 and left uncommitted. Adds app/spike/internal/cmd/integration, which drives the built spike binary end to end against a running `make start` environment rather than importing command internals, so it exercises the seams an operator does. Double-gated so it never runs in the normal suite: the `integration` build tag keeps it out of ordinary builds, and TestMain exits early unless SPIKE_INTEGRATION_TEST=1 confirms a live environment may be probed. Three cases. TestPilotSmokePass covers secret put/get/delete, policy create/get by name, and a cipher round trip, asserting data lands on stdout. TestPilotWarnsWhenNexusUnreachable points one invocation at a closed port, leaving the running Nexus untouched, and asserts the Pilot warns without hanging or panicking; the no-hang assertion doubles as a guard on the open SVID-acquisition-timeout task. Both are non-destructive and clean up after themselves. TestPilotDeniesWhenNexusUninitialized is destructive and gated behind a second flag, SPIKE_INTEGRATION_DESTRUCTIVE=1: reaching a reachable-but-uninitialized Nexus means killing Nexus and every Keeper, losing the in-memory shards, then restarting Nexus alone. It kills the Nexus it spawned, since that process is outside `make start`'s process table and would otherwise hold the port past a Ctrl+C. Both error paths funnel through stdout.HandleAPIError and exit 0 (the subcommands use cobra Run, not RunE), so the assertions key on the stderr message and the no-hang property, not the exit code. Adds `make integration-test` and `make integration-test-destructive`, marks the TASKS.md Phase 3 item done, and records the spec's two open questions as resolved: land in-repo now behind the opt-in gate, and stay complementary to the recovery drill rather than subsuming it. Spec: specs/policy-pattern-anchoring.md Spec: specs/integration-tests.md Signed-off-by: Volkan Özçelik <volkan.ozcelik@broadcom.com> --------- Signed-off-by: Volkan Özçelik <volkan.ozcelik@broadcom.com> Signed-off-by: Volkan Özçelik <volkan.ozcelik@broadcom.com>
1 parent 01954f8 commit 9f33b47

123 files changed

Lines changed: 12147 additions & 18968 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.context/DECISIONS.md

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,48 @@ For significant decisions:
5151
✗ No real alternatives existed
5252
5353
-->
54+
## [2026-07-25-133218] Reserve spike/system/* namespaces against substring-matching policy patterns
55+
56+
**Status**: Accepted
57+
58+
**Context**: A responsible disclosure reported unanchored policy regexes as an over-grant vulnerability. Substring matching by an unanchored regex is documented, intended behavior, but policy management is gated by CheckPolicyAccess against the literal path spike/system/acl, so a PathPattern of 'spike', 'system', or 'acl' matched that gate by substring and conferred control over every policy in the system.
59+
60+
**Decision**: Reserve spike/system/* namespaces against substring-matching policy patterns
61+
62+
**Rationale**: Implicit anchoring (the reporter's proposal) was rejected: it patched only UpsertPolicy while sqlite/persist/regex.go recompiles patterns on every load, so it fixed the memory backend and left SQLite exposed; and wrapping in ^(?:...)$ silently converts working ^-only prefix policies into denials. Instead the three reserved paths now require that a pattern DESCRIBE the path (its full-match form ^(?:p)$ still matches) rather than merely contain it. A purely syntactic ^...$ rule was implemented first and rejected because .* and ^.*$ are the same regex.
63+
64+
**Consequence**: Enforced in both UpsertPolicy (authoring-time rejection) and CheckPolicyAccess (covers pre-existing stored policies and backend recompilation). Ordinary paths keep plain substring semantics. Policies that reached a reserved path via an unanchored pattern now fail loudly. See ADR-0033 and specs/policy-pattern-anchoring.md.
65+
66+
---
67+
68+
## [2026-07-18-110741] Bare-metal harness invokes SPIKE binaries via PATH, deliberately
69+
70+
**Status**: Accepted
71+
72+
**Context**: During the preflight work (2026-07-16) explicit-path invocation was proposed to eliminate name-collision risk with the generic binary names (spike, keeper, demo) and rejected; the rationale was never recorded.
73+
74+
**Decision**: Bare-metal harness invokes SPIKE binaries via PATH, deliberately
75+
76+
**Rationale**: Binaries on PATH are the user-facing convenience, and the harness sharing that resolution forces PATH setup early, keeping one consistent story. The preflight makes collisions loud through shadowing detection instead of eliminating them.
77+
78+
**Consequence**: Do not re-propose explicit-path or prefixed binaries for the dev harness; extend the preflight if new failure modes appear.
79+
80+
---
81+
82+
## [2026-07-17-080305] Config accessors crash fast on missing critical configuration
83+
84+
**Status**: Accepted
85+
86+
**Context**: A jira-era task proposed refactoring the env accessors (KeepersVal and friends) to return sentinel errors instead of calling log.FatalLn; on 2026-07-17 a full SDK brief was drafted for it and withdrawn the same day.
87+
88+
**Decision**: Config accessors crash fast on missing critical configuration
89+
90+
**Rationale**: The crash is intentional: without critical configuration such as SPIKE_NEXUS_KEEPER_PEERS, SPIKE cannot operate reliably, and failing fast beats limping along misconfigured. The accessors are testable through the SPIKE_STACK_TRACES_ON_LOG_FATAL panic-recover pattern, and the env-to-log circular dependency the original task cited dissolved when both packages moved into spike-sdk-go.
91+
92+
**Consequence**: Do not propose returned-error refactors for critical-config accessors in spike or spike-sdk-go. New accessors for must-have configuration should follow the same crash-fast idiom, keeping the panic-mode escape hatch for tests.
93+
94+
---
95+
5496
## [2026-06-13-125427] Pin Go toolchain to 1.26.4 and bump circl/go-jose/x/net to clear govulncheck
5597

5698
**Status**: Accepted

.context/LEARNINGS.md

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,46 @@ DO NOT UPDATE FOR:
2222
<!-- INDEX:END -->
2323

2424
<!-- Add gotchas, tips, and lessons learned here -->
25+
## [2026-07-25-133240] Policy regex patterns are compiled at two sites, not one
26+
27+
**Context**: Evaluating a proposed security patch that anchored policy patterns inside UpsertPolicy. It passed its own test but changed nothing in production.
28+
29+
**Lesson**: UpsertPolicy in app/nexus/internal/state/base/policy.go compiles patterns once at authoring time, but app/nexus/internal/state/backend/sqlite/persist/regex.go recompiles them from the stored strings on EVERY load. CheckPolicyAccess -> ListPolicies -> LoadAllPolicies goes through that second site on every access check. The memory backend retains the struct UpsertPolicy built, so memory-backed tests hide the difference entirely.
30+
31+
**Application**: Any change to how policy patterns are compiled, anchored, or validated must cover both compile sites, or be enforced at CheckPolicyAccess where all paths converge. Always add a SQLite-backed test alongside the memory-backed one; a green memory test proves nothing about production.
32+
33+
---
34+
35+
## [2026-07-18-110741] spiffe.Source without a SPIRE agent hangs forever; a malformed endpoint fails fast
36+
37+
**Context**: The lifecycle integration test hung 148s at RestoreBackingStoreFromPilotShards' source creation: context.Background() with no dial timeout (the open Phase 5 SVID-timeout task, met in the wild).
38+
39+
**Lesson**: SPIFFE_ENDPOINT_SOCKET=bogus://fail-fast makes source creation fail at address validation, instantly and deterministically.
40+
41+
**Application**: Use the malformed endpoint in any test driving code that reaches for a SPIFFE source; pair with SPIKE_STACK_TRACES_ON_LOG_FATAL=true to recover the fatal.
42+
43+
---
44+
45+
## [2026-07-18-110741] SDK config/fs path resolvers are sync.Once-memoized; env overrides must happen in TestMain
46+
47+
**Context**: The test-isolation work found per-test t.Setenv(SPIKE_NEXUS_DATA_DIR, ...) silently ineffective after the first resolution.
48+
49+
**Lesson**: fs.NexusDataFolder and siblings memoize on first call for the process lifetime.
50+
51+
**Application**: Any package whose tests touch SPIKE data or recovery paths needs the env override in TestMain before m.Run(), one temporary directory per package run.
52+
53+
---
54+
55+
## [2026-07-18-110741] make audit lint runs with CGO_ENABLED=0, so typed sqlite error inspection breaks it
56+
57+
**Context**: The sqlite retry work used sqlite3.Error/ErrBusy from mattn/go-sqlite3; build and tests passed, then the gate failed: golangci-lint typechecks with CGO off, where mattn's typed API does not exist.
58+
59+
**Lesson**: Detect transient sqlite failures by driver error strings (database is locked / database table is locked), never by mattn types.
60+
61+
**Application**: Keep mattn/go-sqlite3 symbol references out of files that are not cgo-gated; anything else fails make audit.
62+
63+
---
64+
2565
## [2026-06-13-170816] SPIKE k8s integration test was missing keeper bootstrap; plus a verify-path deadlock
2666

2767
**Context**: minio-rolearn integration test (CI red on main, pre-existing) hangs because keepers are never seeded with root-key shares; SPIKE Nexus InitializeBackingStoreFromKeepers waits forever (retry.Forever, by design until keepers are hydrated). The spire helm chart registers the spike/bootstrap identity but ships no bootstrap workload, and hack/k8s/Bootstrap.yaml does not exist.

.context/TASKS.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ the name-based policy work.
4040
- [x] Fix `spike cipher` stream mode (broken; owner: Murat); JSON mode fix unblocks encryption-as-a-service demo/docs #source:jira.xml #added:2026-07-14 #done:2026-07-15 (stale: both cipher streaming and file modes verified passing via the make start checks on 2026-07-15)
4141
- [x] Retry sqlite operations with exponential backoff on transient locks → ideas/research-db-resilience.md #source:jira.xml #added:2026-07-14 #done:2026-07-16 (withSerializableTx retries SQLITE_BUSY/SQLITE_LOCKED with exponential backoff at the single choke point every write flows through; reads rely on WAL plus the busy_timeout DSN parameter and honor the operation deadline; note the DB ops live under state/backend/sqlite/persist these days, not state/persist)
4242
- [x] Bound the Bootstrap keeper-wait loop with a configurable timeout/max-attempts instead of looping forever → ideas/research-db-resilience.md #source:jira.xml #added:2026-07-14 #done:2026-07-16 (stale: superseded by the SDK retry migration; broadcastToKeeper bounds each keeper with retry.WithMaxAttempts, a per-keeper context timeout, and configurable backoff intervals — app/bootstrap/internal/net/dispatch.go — and broadcast.go bounds init verification with WithMaxElapsedTime)
43-
- [ ] Make `env` accessors return sentinel errors instead of calling `log.FatalLn` (removes env→log circular dep, makes them testable) → ideas/research-env-error-handling.md #source:jira.xml #added:2026-07-14 (2026-07-16: the offending accessors now live in spike-sdk-go config/env, so the sentinel-error refactor is an upstream SDK change; the in-repo share is adapting callers once the SDK ships it)
43+
- [-] Make `env` accessors return sentinel errors instead of calling `log.FatalLn` ideas/research-env-error-handling.md #source:jira.xml #added:2026-07-14 #skipped:2026-07-17 (working as intended: the crash-fast behavior on missing critical config such as SPIKE_NEXUS_KEEPER_PEERS is deliberate, since SPIKE cannot operate reliably without it; the accessors ARE testable via the SPIKE_STACK_TRACES_ON_LOG_FATAL panic-recover pattern; and the env-to-log circular dependency the jira item cited dissolved when both packages moved into spike-sdk-go)
4444
- [x] Fix Pilot printing normal output to stderr: cobra Print* writes to OutOrStderr and the root command never calls SetOut, so data output (secrets included) lands on stderr and `spike secret get x > file.txt` yields an empty file; every harness script compensates with 2>&1. #added:2026-07-16 #done:2026-07-16 (rootCmd.SetOut(os.Stdout) in cmd.Initialize; PrintErr still goes to stderr, and the harness scripts that merge streams keep working)
4545

4646
### Phase 2: SDK Extraction `#priority:medium`
@@ -49,7 +49,7 @@ the name-based policy work.
4949
### Phase 3: Testing `#priority:medium`
5050
- [x] Make `make test` concurrent again → ideas/research-cli-testing.md #source:jira.xml #added:2026-07-14 #done:2026-07-17 (removed -p 1 once the data-dir isolation landed; nothing else shared state across packages — no fixed ports, no t.Parallel, env vars are per-process. Full -race suite: 29.7s serialized to 2.9s concurrent, roughly 10x; two consecutive concurrent runs clean)
5151
- [x] Move the sqlite state tests off the real ~/.spike/data/spike.db: make test deleted the live dev environment database mid-run (bit us three times this week). #added:2026-07-16 #done:2026-07-16 (fs.NexusDataFolder is sync.Once-memoized, so per-test t.Setenv cannot work; instead each affected package sets SPIKE_NEXUS_DATA_DIR to a per-run temp dir in TestMain before the first resolution — state/base, state/persist, and backend/sqlite/persist — verified: a full package run leaves ~/.spike untouched)
52-
- [ ] Add integration tests: root key cached/recovered/not-re-initialized; secret & policy CRUD; Pilot denies when Nexus uninitialized / warns when unreachable → ideas/research-cli-testing.md #source:jira.xml #added:2026-07-14 #in-progress (2026-07-17: Slice A shipped — specs/integration-tests.md; app/nexus/internal/state/integration covers the root-key lifecycle, the not-re-initialized-twice invariant, CRUD, and an in-process shard-restore round trip inside the normal suite. Remaining: Slice B, the Pilot uninitialized/unreachable behaviors, gated on the spec open questions)
52+
- [x] Add integration tests: root key cached/recovered/not-re-initialized; secret & policy CRUD; Pilot denies when Nexus uninitialized / warns when unreachable → ideas/research-cli-testing.md #source:jira.xml #added:2026-07-14 #done:2026-07-18 (2026-07-17: Slice A shipped — specs/integration-tests.md; app/nexus/internal/state/integration covers the root-key lifecycle, the not-re-initialized-twice invariant, CRUD, and an in-process shard-restore round trip inside the normal suite. 2026-07-18: Slice B shipped — app/spike/internal/cmd/integration, build tag integration + SPIKE_INTEGRATION_TEST=1, drives the spike binary end to end. Non-destructive CRUD+cipher smoke pass (asserts secret data on stdout) and the Nexus-unreachable warning both verified live against make start; the uninitialized-Nexus denial is a destructive, double-gated test (SPIKE_INTEGRATION_DESTRUCTIVE=1) reusing the drill's kill/restart-Nexus-alone machinery; it cleans up the Nexus it spawns, so a Ctrl+C on the make start terminal (or make kill) then make start resets the env. Spec open questions resolved: land now opt-in, complementary to the drill. make targets: integration-test / integration-test-destructive)
5353
- [ ] Raise CLI command coverage to 60%+ via unit + HTTP-mock tests; fix `t.Skip()`ed tests; DI-refactor `sendShardsToKeepers` → ideas/research-cli-testing.md #source:jira.xml #added:2026-07-14
5454
- [x] `start.sh` should exercise recovery/restore and encryption/decryption #source:jira.xml #added:2026-07-14 #done:2026-07-16 (encryption/decryption checks live in start.sh since the policy-validation rework; recovery/restore is exercised by make drill-recovery, kept as a separate second-terminal script deliberately so the crash simulation never runs inside the normal startup path)
5555
- [x] Scripted live recovery/restore drill: once `make start` completes cleanly, run `spike operator recover`, kill Nexus and the Keepers, restart Nexus alone, feed the shards back via `spike operator restore` (scriptable via stdin since fix/operator-restore), and verify a pre-crash secret reads back. Rationale: the 2026-07-16 code review found no live breakage (shard-index fidelity intact end to end; guards use exact SPIFFE role matching, unaffected by the policy-name migration), so only a drill can prove the Phase 1 "recovery/restore is broken" claim stale and close both tasks. Needs the recover/restore role entries (spire-server-entry-recover-register.sh / -restore-register.sh), which make start does not register by default. #added:2026-07-16 #done:2026-07-16 (implemented as hack/bare-metal/drill/recovery-drill.sh behind make drill-recovery; the drill first exposed the Nexus boot-order deadlock, then passed end to end once it was fixed)

CLAUDE.md

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,9 +177,31 @@ https://ctx.ist/recipes/build-a-knowledge-base/.
177177
SPIKE Policies use `SPIFFEIDPattern` and `PathPattern` fields. Those fields
178178
are regular expression Strings; NOT globs.
179179

180-
- **For Policy SPIFFEID and Path patterns, ALWAYS use regex patterns, NOT globs**
181-
- ✅ Correct: `/path/to/.*`, `spiffe://example\.org/workload/.*`
182-
- ❌ Wrong: `/path/to/*`, `spiffe://example.org/workload/*`
180+
Two rules apply, and both matter:
181+
182+
1. **Use regex syntax, not glob syntax.** Write `.*`, never `*`, and escape
183+
literal dots.
184+
2. **Anchor both ends with `^` and `$`.** These patterns are matched with
185+
`regexp.MatchString`, which succeeds on a substring match. SPIKE compiles
186+
what you wrote and adds nothing. An unanchored pattern grants access far
187+
beyond what it appears to say: `app/config` also matches
188+
`private-app/configs/master-key`, and `spiffe://example\.org/app` also
189+
matches `spiffe://example.org/app-attacker`.
190+
191+
- ✅ Correct: `^path/to/.*$`, `^spiffe://example\.org/workload/.*$`
192+
- ❌ Wrong (glob): `path/to/*`, `spiffe://example.org/workload/*`
193+
- ❌ Wrong (unanchored): `path/to/.*`, `spiffe://example\.org/workload/.*`
194+
195+
Prefer the narrowest pattern that satisfies the requirement: an exact path
196+
first, then a bounded subtree, and treat a broad wildcard as something that
197+
needs justifying.
198+
199+
SPIKE reserves three internal namespaces (`spike/system/acl`,
200+
`spike/system/secret`, `spike/system/cipher/exec`). A policy that reaches
201+
one of them only through substring matching is rejected, because write
202+
access to `spike/system/acl` confers control over every policy in the
203+
system. To grant system access deliberately, spell the path out and anchor
204+
it: `^spike/system/acl$`.
183205

184206
### Paths used in Secrets and Policies are NOT Unix-like paths; they are Namespaces
185207

app/nexus/internal/route/acl/policy/get.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ import (
4646
// {
4747
// "policy": {
4848
// "name": "example-policy",
49-
// "spiffe_id_pattern": "^spiffe://example\.org/.*/service",
50-
// "path_pattern": "^api/",
49+
// "spiffe_id_pattern": "^spiffe://example\.org/.*/service$",
50+
// "path_pattern": "^api/.*$",
5151
// "permissions": ["read", "write"],
5252
// "created_at": "2024-01-01T00:00:00Z",
5353
// "created_by": "user-abc"

app/nexus/internal/state/base/policy.go

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,16 @@ import (
4545
//
4646
// A policy matches when its SPIFFE ID pattern matches the requestor's ID and
4747
// its path pattern matches the requested path.
48+
//
49+
// Policy patterns are ordinary Go regular expressions and therefore match on
50+
// any substring unless the author anchors them. That is intended, and it
51+
// applies to every ordinary path. SPIKE's reserved system namespaces
52+
// (spike/system/acl, spike/system/secret, spike/system/cipher/exec) are the
53+
// exception: a policy grants access to those only when it describes the path
54+
// rather than merely containing it, and when its SPIFFE ID pattern is precise
55+
// enough to name the identities it covers. A policy that reaches a reserved
56+
// path only because its pattern appears there as a substring is ignored, and
57+
// the refusal is logged. See guardReservedSystemPaths.
4858
func CheckPolicyAccess(
4959
peerSPIFFEID string, path string, wants []data.PolicyPermission,
5060
) bool {
@@ -67,6 +77,21 @@ func CheckPolicyAccess(
6777
continue
6878
}
6979

80+
// Reserved system paths gate SPIKE's own privileged operations. A
81+
// policy reaches them only by describing them deliberately; a pattern
82+
// that merely contains one as a substring must not confer privileged
83+
// access. This is enforced here as well as in UpsertPolicy so that
84+
// policies stored before the rule existed, and policies recompiled by
85+
// a backend on load, are both covered.
86+
if isReservedSystemPath(path) &&
87+
!policyMayReachReservedPath(policy, path) {
88+
log.Warn(fName,
89+
"message", "ignoring substring-only policy for reserved path",
90+
"policy", policy.Name, "path", path,
91+
)
92+
continue
93+
}
94+
7095
if validation.ValidatePolicyPermissions(policy.Permissions, wants) {
7196
return true
7297
}
@@ -90,12 +115,15 @@ func CheckPolicyAccess(
90115
//
91116
// Returns:
92117
// - data.Policy: The created or updated policy, including timestamps
93-
// - *sdkErrors.SDKError: ErrEntityInvalid if the policy name is empty or
94-
// regex patterns are invalid, ErrEntityLoadFailed or ErrEntitySaveFailed
95-
// for backend errors
118+
// - *sdkErrors.SDKError: ErrEntityInvalid if the policy name is empty, the
119+
// regex patterns are invalid, or the policy reaches a reserved system
120+
// path only by substring match; ErrEntityLoadFailed or
121+
// ErrEntitySaveFailed for backend errors
96122
//
97123
// The function performs the following:
98124
// - Compiles and stores regex patterns for SPIFFEIDPattern and PathPattern
125+
// - Rejects policies that reach a reserved system namespace without
126+
// describing it deliberately (see guardReservedSystemPaths)
99127
// - For new policies: sets CreatedAt and UpdatedAt
100128
// - For existing policies: preserves CreatedAt, updates UpdatedAt
101129
func UpsertPolicy(policy data.Policy) (data.Policy, *sdkErrors.SDKError) {
@@ -133,6 +161,12 @@ func UpsertPolicy(policy data.Policy) (data.Policy, *sdkErrors.SDKError) {
133161
}
134162
policy.PathRegex = pathRegex
135163

164+
// Refuse to store a policy that would reach a reserved system namespace
165+
// without anchoring both patterns. Ordinary paths are unaffected.
166+
if guardErr := guardReservedSystemPaths(policy); guardErr != nil {
167+
return data.Policy{}, guardErr
168+
}
169+
136170
now := time.Now()
137171

138172
if existingPolicy != nil {

0 commit comments

Comments
 (0)