feat: add control plane principal scope#101
Conversation
|
Warning Review limit reached
Next review available in: 30 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis PR introduces a protocol-neutral ChangesPrincipal Scope Attribution
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Fix the 5 golangci-lint issues that failed the QA workflow: forcetypeassert on the auxreq.Client type assertions, modernize (slices.ContainsFunc for roles sanitization and reflect.Type.Fields iterator in the view test), and staticcheck QF1011 replaced with a compile-time guard. No behavior change. Co-authored-by: Cursor <cursoragent@cursor.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
internal/stdhttp/auth/adapter.go (1)
96-133: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winClear
d.Scopebefore renderer hooks.internal/stdhttp/auth/adapter.go:106-133The built-in renderer ignoresDecision.Scope, butcallRendererstill passes the full decision to the publicAuthErrorRendererhook. If a custom renderer inspectsDecision.Scope, the unsafe scope frombridgeScopecan reach the response path.d.Scope = nilhere keeps the extension point safe.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/stdhttp/auth/adapter.go` around lines 96 - 133, Clear the decision scope before any renderer hook is invoked in the auth adapter flow. In the bridgeScope/authDecisionEvent path, if bridgeScope returns an error and d is downgraded to deny, explicitly set d.Scope to nil before calling p.callRenderer so custom AuthErrorRenderer implementations cannot see unsafe scope material. Keep the existing allow/challenge/deny handling in adapter.go, but ensure the sanitized decision is what reaches the renderer.pkg/lipsdk/traffic/observe.go (1)
63-73: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
CaptureMetais no longer comparable.Adding
Scope scope.PrincipalScopeViewmakes this exported SDK type non-comparable becausePrincipalScopeViewcontains slices and maps. Any downstream code that usesCaptureMetawith==or as a map key will stop compiling. If comparability is meant to remain part of the contract, keepScopeout of the value type (for example, store a pointer or a separate wrapper).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/lipsdk/traffic/observe.go` around lines 63 - 73, CaptureMeta became non-comparable after adding the Scope field, so downstream uses with == or as map keys will break. Update the CaptureMeta type in observe.go to preserve comparability by removing the direct scope.PrincipalScopeView value field and replacing it with a comparable alternative such as a pointer or moving Scope into a separate wrapper type. Keep the exported API compatible by ensuring CaptureMeta itself remains comparable.Source: Path instructions
pkg/lipsdk/usage/observe.go (1)
11-26: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
usage.Eventno longer stays comparable. AddingScope scope.PrincipalScopeViewmakes the exported type non-comparable, so downstream code that uses==or map keys withusage.Eventwill stop compiling. If this is intentional, call out the API break; otherwise keepScopebehind a pointer or another comparable wrapper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/lipsdk/usage/observe.go` around lines 11 - 26, usage.Event has become non-comparable because Event now embeds Scope scope.PrincipalScopeView, which can break downstream equality checks and map keys; update the Event struct to preserve comparability by storing Scope behind a comparable indirection (for example, a pointer or other comparable wrapper) or, if the API break is intended, adjust the exported contract accordingly. Make the fix in usage.Event in observe.go and ensure any code constructing or reading Event still handles Scope correctly.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/core/auth/scope.go`:
- Around line 97-166: The credential filter in SanitizeScope is too narrow,
since looksCredentialLike only matches a small set of exact substrings and
misses common raw secret formats. Broaden the heuristic in looksCredentialLike
to catch additional secret-like values such as JWT-shaped strings, API key
prefixes, and separator variants like password: or secret=, or explicitly
tighten the SanitizeScope doc comment to state it is best-effort only. Keep the
changes centered on SanitizeScope, looksCredentialLike, and
credentialLikePatterns so callers don’t over-rely on this gate.
- Around line 39-50: BuildScope currently lets the trusted-scope path succeed
even when the resulting Scope has no real Principal.ID, which violates the same
identity requirement enforced by the legacy-principal branch. Update BuildScope
in scope.go to validate the cloned trusted scope after SanitizeScope and return
ErrNoIdentity when s.Principal() is empty, matching the behavior already used
for the legacy path. Add or update a scope_test.go case around BuildScope to
cover the trusted-scope-without-identity scenario and assert it fails with
ErrNoIdentity.
In `@internal/core/config/access_auth_local_attribution_test.go`:
- Around line 147-167: The test named
TestValidateAuthLocalAPIKeyRecords_attributionConverted is bypassing the config
conversion path and validating core records directly, so it does not exercise
toCore() or config.ValidateAuthLocalAPIKeyRecords. Update the test to call
config.ValidateAuthLocalAPIKeyRecords with the local API key attribution fixture
and assert the resulting core auth record behavior indirectly, rather than
manually constructing coreauth.LocalAPIKeyRecord. Make sure the test verifies
the full conversion path, including fields like AuthMethod, OrganizationID,
WorkspaceID, ProjectID, DepartmentID, CostCenterID, PolicyLabels, and
Attribution.
In `@internal/core/config/access_auth_validate.go`:
- Around line 93-107: The issue is that AuthLocalAttribution.toCore() passes
Roles, SafeClaims, and PolicyLabels by reference, letting
coreauth.LocalAttribution alias mutable config state. Update toCore() to
deep-copy those slice/map fields before returning, and verify
NewLocalAPIKeyAuthenticator in local_apikey.go does not keep the shared
references. Keep the fix centered on AuthLocalAttribution.toCore and the
LocalAPIKeyAuthenticator constructor so later mutations cannot bypass
validateLocalAttribution.
In `@internal/stdhttp/auth/adapter.go`:
- Around line 102-108: The forced deny path in the auth adapter is reusing any
preexisting ReasonCode, which can leave a stale Allow-era code attached to an
unsafe-scope rejection. Update the bridged.err branch in auth/adapter.go so that
when d.Outcome is set to auth.OutcomeDeny, d.ReasonCode is also reset to the
unsafe-scope value unconditionally (or otherwise cleared before assigning it),
ensuring defaultTerminalHTTPStatus and the rendered error always reflect the
actual unsafe-scope denial.
In `@pkg/lipsdk/scope/context.go`:
- Around line 5-7: The context key definition in ctxKey/keyScope uses a magic
iota offset without any local functional need. Update keyScope in context.go to
use a plain iota unless there is a real cross-package key registry convention;
if the offset must remain, add a short comment near keyScope explaining the
shared-ID convention. Keep the change focused on the ctxKey and keyScope symbols
so the intent is clear.
---
Outside diff comments:
In `@internal/stdhttp/auth/adapter.go`:
- Around line 96-133: Clear the decision scope before any renderer hook is
invoked in the auth adapter flow. In the bridgeScope/authDecisionEvent path, if
bridgeScope returns an error and d is downgraded to deny, explicitly set d.Scope
to nil before calling p.callRenderer so custom AuthErrorRenderer implementations
cannot see unsafe scope material. Keep the existing allow/challenge/deny
handling in adapter.go, but ensure the sanitized decision is what reaches the
renderer.
In `@pkg/lipsdk/traffic/observe.go`:
- Around line 63-73: CaptureMeta became non-comparable after adding the Scope
field, so downstream uses with == or as map keys will break. Update the
CaptureMeta type in observe.go to preserve comparability by removing the direct
scope.PrincipalScopeView value field and replacing it with a comparable
alternative such as a pointer or moving Scope into a separate wrapper type. Keep
the exported API compatible by ensuring CaptureMeta itself remains comparable.
In `@pkg/lipsdk/usage/observe.go`:
- Around line 11-26: usage.Event has become non-comparable because Event now
embeds Scope scope.PrincipalScopeView, which can break downstream equality
checks and map keys; update the Event struct to preserve comparability by
storing Scope behind a comparable indirection (for example, a pointer or other
comparable wrapper) or, if the API break is intended, adjust the exported
contract accordingly. Make the fix in usage.Event in observe.go and ensure any
code constructing or reading Event still handles Scope correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c6ee3c5d-2b6c-4d68-a7cf-50afc5942a74
📒 Files selected for processing (56)
.kiro/specs/control-plane-principal-scope/tasks.mddocs/dogfood-local.mdinternal/archtest/scope_boundary_test.gointernal/core/auth/errors.gointernal/core/auth/local_apikey.gointernal/core/auth/local_apikey_record.gointernal/core/auth/local_apikey_scope_test.gointernal/core/auth/local_noop.gointernal/core/auth/local_noop_scope_test.gointernal/core/auth/scope.gointernal/core/auth/scope_phase6_attribution_only_test.gointernal/core/auth/scope_test.gointernal/core/auxreq/client.gointernal/core/auxreq/scope_test.gointernal/core/config/access_auth_local_attribution_test.gointernal/core/config/access_auth_model.gointernal/core/config/access_auth_validate.gointernal/core/execctx/views.gointernal/core/execctx/views_scope_test.gointernal/core/runtime/attempt_stream.gointernal/core/runtime/attempt_stream_scope_test.gointernal/core/runtime/executor_open_attempt.gointernal/core/runtime/executor_prepare_secure.gointernal/core/runtime/executor_scope_test.gointernal/core/runtime/executor_secure_session_test.gointernal/core/runtime/scope_phase6_compatibility_test.gointernal/core/runtime/scope_phase6_secret_safety_test.gointernal/core/runtime/scope_resolver.gointernal/core/runtime/scope_resolver_test.gointernal/core/runtime/secure_session.gointernal/stdhttp/auth/adapter.gointernal/stdhttp/auth/adapter_test.gointernal/stdhttp/auth/middleware.gointernal/stdhttp/auth/middleware_test.gointernal/stdhttp/auth/scope_bridge_test.gopkg/lipsdk/auth/decision.gopkg/lipsdk/auth/decision_scope_test.gopkg/lipsdk/auth/events.gopkg/lipsdk/auth/events_scope_test.gopkg/lipsdk/execview/views.gopkg/lipsdk/scope/context.gopkg/lipsdk/scope/context_test.gopkg/lipsdk/scope/doc.gopkg/lipsdk/scope/value.gopkg/lipsdk/scope/value_test.gopkg/lipsdk/scope/view.gopkg/lipsdk/scope/view_test.gopkg/lipsdk/traffic/emit.gopkg/lipsdk/traffic/observe.gopkg/lipsdk/traffic/observe_scope_test.gopkg/lipsdk/transport/httpauth/context.gopkg/lipsdk/transport/httpauth/context_scope_test.gopkg/lipsdk/transport/httpauth/result.gopkg/lipsdk/transport/httpauth/result_scope_test.gopkg/lipsdk/usage/observe.gopkg/lipsdk/usage/observe_scope_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
pkg/lipsdk/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
pkg/lipsdk/**/*.go: Inpkg/lipsdk/, keep plugin SDK, facades, and registration contracts.
Keep publicpkg/lipsdkcontracts minimal, documented, and versionable.
Files:
pkg/lipsdk/auth/decision_scope_test.gopkg/lipsdk/transport/httpauth/context_scope_test.gopkg/lipsdk/transport/httpauth/result_scope_test.gopkg/lipsdk/scope/doc.gopkg/lipsdk/auth/events.gopkg/lipsdk/transport/httpauth/context.gopkg/lipsdk/scope/value.gopkg/lipsdk/execview/views.gopkg/lipsdk/traffic/emit.gopkg/lipsdk/scope/context.gopkg/lipsdk/usage/observe.gopkg/lipsdk/transport/httpauth/result.gopkg/lipsdk/scope/context_test.gopkg/lipsdk/traffic/observe.gopkg/lipsdk/auth/decision.gopkg/lipsdk/scope/value_test.gopkg/lipsdk/usage/observe_scope_test.gopkg/lipsdk/traffic/observe_scope_test.gopkg/lipsdk/auth/events_scope_test.gopkg/lipsdk/scope/view_test.gopkg/lipsdk/scope/view.go
**/*.go
📄 CodeRabbit inference engine (Custom checks)
**/*.go: For server, CLI, worker, or network Go code, ensurecontext.Contextis propagated correctly, cancellation is respected, and new goroutines cannot leak indefinitely.
Do not make accidental public API breaks in Go code: underpkg/**or anywhere exported Go identifiers are changed, warn if the PR changes exported types, function signatures, error behavior, JSON fields, CLI flags, config keys, or documented behavior without clearly explaining the compatibility impact.
Files:
pkg/lipsdk/auth/decision_scope_test.gopkg/lipsdk/transport/httpauth/context_scope_test.gointernal/core/auxreq/scope_test.gopkg/lipsdk/transport/httpauth/result_scope_test.gopkg/lipsdk/scope/doc.gointernal/core/execctx/views_scope_test.gopkg/lipsdk/auth/events.gopkg/lipsdk/transport/httpauth/context.gopkg/lipsdk/scope/value.gointernal/stdhttp/auth/adapter_test.gopkg/lipsdk/execview/views.gopkg/lipsdk/traffic/emit.gopkg/lipsdk/scope/context.gointernal/archtest/scope_boundary_test.gointernal/core/runtime/secure_session.gopkg/lipsdk/usage/observe.gopkg/lipsdk/transport/httpauth/result.gointernal/core/auth/local_apikey_scope_test.gointernal/core/auth/errors.gointernal/core/auxreq/client.gopkg/lipsdk/scope/context_test.gointernal/core/runtime/executor_open_attempt.gopkg/lipsdk/traffic/observe.gointernal/core/execctx/views.gointernal/core/auth/local_noop_scope_test.gointernal/core/runtime/executor_secure_session_test.gopkg/lipsdk/auth/decision.gointernal/core/runtime/scope_resolver_test.gointernal/core/runtime/scope_resolver.gopkg/lipsdk/scope/value_test.gointernal/core/auth/scope_phase6_attribution_only_test.gointernal/core/config/access_auth_local_attribution_test.gointernal/stdhttp/auth/middleware.gopkg/lipsdk/usage/observe_scope_test.gointernal/core/auth/scope.gopkg/lipsdk/traffic/observe_scope_test.gopkg/lipsdk/auth/events_scope_test.gointernal/core/runtime/scope_phase6_compatibility_test.gopkg/lipsdk/scope/view_test.gointernal/core/auth/local_noop.gointernal/core/auth/local_apikey.gointernal/core/runtime/executor_scope_test.gointernal/core/config/access_auth_validate.gointernal/core/config/access_auth_model.gointernal/core/runtime/executor_prepare_secure.gointernal/core/auth/local_apikey_record.gointernal/core/runtime/scope_phase6_secret_safety_test.gointernal/core/runtime/attempt_stream_scope_test.gointernal/stdhttp/auth/adapter.gointernal/stdhttp/auth/middleware_test.gointernal/core/runtime/attempt_stream.gointernal/core/auth/scope_test.gointernal/stdhttp/auth/scope_bridge_test.gopkg/lipsdk/scope/view.go
⚙️ CodeRabbit configuration file
**/*.go: Review as production Go code. Prioritize correctness, race conditions, goroutine leaks, context cancellation, timeout handling, error wrapping, nil-pointer risks, resource cleanup, defer placement, API compatibility, interface design, dependency boundaries, and testability. Avoid generic style comments when gofmt/golangci-lint already covers the issue.
Files:
pkg/lipsdk/auth/decision_scope_test.gopkg/lipsdk/transport/httpauth/context_scope_test.gointernal/core/auxreq/scope_test.gopkg/lipsdk/transport/httpauth/result_scope_test.gopkg/lipsdk/scope/doc.gointernal/core/execctx/views_scope_test.gopkg/lipsdk/auth/events.gopkg/lipsdk/transport/httpauth/context.gopkg/lipsdk/scope/value.gointernal/stdhttp/auth/adapter_test.gopkg/lipsdk/execview/views.gopkg/lipsdk/traffic/emit.gopkg/lipsdk/scope/context.gointernal/archtest/scope_boundary_test.gointernal/core/runtime/secure_session.gopkg/lipsdk/usage/observe.gopkg/lipsdk/transport/httpauth/result.gointernal/core/auth/local_apikey_scope_test.gointernal/core/auth/errors.gointernal/core/auxreq/client.gopkg/lipsdk/scope/context_test.gointernal/core/runtime/executor_open_attempt.gopkg/lipsdk/traffic/observe.gointernal/core/execctx/views.gointernal/core/auth/local_noop_scope_test.gointernal/core/runtime/executor_secure_session_test.gopkg/lipsdk/auth/decision.gointernal/core/runtime/scope_resolver_test.gointernal/core/runtime/scope_resolver.gopkg/lipsdk/scope/value_test.gointernal/core/auth/scope_phase6_attribution_only_test.gointernal/core/config/access_auth_local_attribution_test.gointernal/stdhttp/auth/middleware.gopkg/lipsdk/usage/observe_scope_test.gointernal/core/auth/scope.gopkg/lipsdk/traffic/observe_scope_test.gopkg/lipsdk/auth/events_scope_test.gointernal/core/runtime/scope_phase6_compatibility_test.gopkg/lipsdk/scope/view_test.gointernal/core/auth/local_noop.gointernal/core/auth/local_apikey.gointernal/core/runtime/executor_scope_test.gointernal/core/config/access_auth_validate.gointernal/core/config/access_auth_model.gointernal/core/runtime/executor_prepare_secure.gointernal/core/auth/local_apikey_record.gointernal/core/runtime/scope_phase6_secret_safety_test.gointernal/core/runtime/attempt_stream_scope_test.gointernal/stdhttp/auth/adapter.gointernal/stdhttp/auth/middleware_test.gointernal/core/runtime/attempt_stream.gointernal/core/auth/scope_test.gointernal/stdhttp/auth/scope_bridge_test.gopkg/lipsdk/scope/view.go
**/*
📄 CodeRabbit inference engine (Custom checks)
Do not introduce hardcoded credentials, API keys, tokens, private keys, passwords, production secrets, or sensitive internal URLs.
Files:
pkg/lipsdk/auth/decision_scope_test.gopkg/lipsdk/transport/httpauth/context_scope_test.gointernal/core/auxreq/scope_test.gopkg/lipsdk/transport/httpauth/result_scope_test.gopkg/lipsdk/scope/doc.gointernal/core/execctx/views_scope_test.gopkg/lipsdk/auth/events.gopkg/lipsdk/transport/httpauth/context.gopkg/lipsdk/scope/value.gointernal/stdhttp/auth/adapter_test.gopkg/lipsdk/execview/views.gopkg/lipsdk/traffic/emit.gopkg/lipsdk/scope/context.gointernal/archtest/scope_boundary_test.gointernal/core/runtime/secure_session.gopkg/lipsdk/usage/observe.gopkg/lipsdk/transport/httpauth/result.gointernal/core/auth/local_apikey_scope_test.gointernal/core/auth/errors.gointernal/core/auxreq/client.gopkg/lipsdk/scope/context_test.gointernal/core/runtime/executor_open_attempt.gopkg/lipsdk/traffic/observe.gointernal/core/execctx/views.gointernal/core/auth/local_noop_scope_test.gointernal/core/runtime/executor_secure_session_test.gopkg/lipsdk/auth/decision.godocs/dogfood-local.mdinternal/core/runtime/scope_resolver_test.gointernal/core/runtime/scope_resolver.gopkg/lipsdk/scope/value_test.gointernal/core/auth/scope_phase6_attribution_only_test.gointernal/core/config/access_auth_local_attribution_test.gointernal/stdhttp/auth/middleware.gopkg/lipsdk/usage/observe_scope_test.gointernal/core/auth/scope.gopkg/lipsdk/traffic/observe_scope_test.gopkg/lipsdk/auth/events_scope_test.gointernal/core/runtime/scope_phase6_compatibility_test.gopkg/lipsdk/scope/view_test.gointernal/core/auth/local_noop.gointernal/core/auth/local_apikey.gointernal/core/runtime/executor_scope_test.gointernal/core/config/access_auth_validate.gointernal/core/config/access_auth_model.gointernal/core/runtime/executor_prepare_secure.gointernal/core/auth/local_apikey_record.gointernal/core/runtime/scope_phase6_secret_safety_test.gointernal/core/runtime/attempt_stream_scope_test.gointernal/stdhttp/auth/adapter.gointernal/stdhttp/auth/middleware_test.gointernal/core/runtime/attempt_stream.gointernal/core/auth/scope_test.gointernal/stdhttp/auth/scope_bridge_test.gopkg/lipsdk/scope/view.go
pkg/**
⚙️ CodeRabbit configuration file
pkg/**: Treat exported identifiers as public API. Flag breaking changes, ambiguous contracts, missing error semantics, poor interface boundaries, and changes that make downstream usage harder.
Files:
pkg/lipsdk/auth/decision_scope_test.gopkg/lipsdk/transport/httpauth/context_scope_test.gopkg/lipsdk/transport/httpauth/result_scope_test.gopkg/lipsdk/scope/doc.gopkg/lipsdk/auth/events.gopkg/lipsdk/transport/httpauth/context.gopkg/lipsdk/scope/value.gopkg/lipsdk/execview/views.gopkg/lipsdk/traffic/emit.gopkg/lipsdk/scope/context.gopkg/lipsdk/usage/observe.gopkg/lipsdk/transport/httpauth/result.gopkg/lipsdk/scope/context_test.gopkg/lipsdk/traffic/observe.gopkg/lipsdk/auth/decision.gopkg/lipsdk/scope/value_test.gopkg/lipsdk/usage/observe_scope_test.gopkg/lipsdk/traffic/observe_scope_test.gopkg/lipsdk/auth/events_scope_test.gopkg/lipsdk/scope/view_test.gopkg/lipsdk/scope/view.go
**/*_test.go
⚙️ CodeRabbit configuration file
**/*_test.go: Review tests for meaningful assertions, table-driven coverage, race-prone tests, t.Parallel misuse, nondeterminism, leaked goroutines, real network or filesystem dependencies, fragile sleeps, and missing edge cases. Prefer testing observable behavior over implementation details.
Files:
pkg/lipsdk/auth/decision_scope_test.gopkg/lipsdk/transport/httpauth/context_scope_test.gointernal/core/auxreq/scope_test.gopkg/lipsdk/transport/httpauth/result_scope_test.gointernal/core/execctx/views_scope_test.gointernal/stdhttp/auth/adapter_test.gointernal/archtest/scope_boundary_test.gointernal/core/auth/local_apikey_scope_test.gopkg/lipsdk/scope/context_test.gointernal/core/auth/local_noop_scope_test.gointernal/core/runtime/executor_secure_session_test.gointernal/core/runtime/scope_resolver_test.gopkg/lipsdk/scope/value_test.gointernal/core/auth/scope_phase6_attribution_only_test.gointernal/core/config/access_auth_local_attribution_test.gopkg/lipsdk/usage/observe_scope_test.gopkg/lipsdk/traffic/observe_scope_test.gopkg/lipsdk/auth/events_scope_test.gointernal/core/runtime/scope_phase6_compatibility_test.gopkg/lipsdk/scope/view_test.gointernal/core/runtime/executor_scope_test.gointernal/core/runtime/scope_phase6_secret_safety_test.gointernal/core/runtime/attempt_stream_scope_test.gointernal/stdhttp/auth/middleware_test.gointernal/core/auth/scope_test.gointernal/stdhttp/auth/scope_bridge_test.go
internal/core/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
In
internal/core/, keep runtime orchestration, routing, continuity, streams, hooks/extensions, config, and diagnostics.
Files:
internal/core/auxreq/scope_test.gointernal/core/execctx/views_scope_test.gointernal/core/runtime/secure_session.gointernal/core/auth/local_apikey_scope_test.gointernal/core/auth/errors.gointernal/core/auxreq/client.gointernal/core/runtime/executor_open_attempt.gointernal/core/execctx/views.gointernal/core/auth/local_noop_scope_test.gointernal/core/runtime/executor_secure_session_test.gointernal/core/runtime/scope_resolver_test.gointernal/core/runtime/scope_resolver.gointernal/core/auth/scope_phase6_attribution_only_test.gointernal/core/config/access_auth_local_attribution_test.gointernal/core/auth/scope.gointernal/core/runtime/scope_phase6_compatibility_test.gointernal/core/auth/local_noop.gointernal/core/auth/local_apikey.gointernal/core/runtime/executor_scope_test.gointernal/core/config/access_auth_validate.gointernal/core/config/access_auth_model.gointernal/core/runtime/executor_prepare_secure.gointernal/core/auth/local_apikey_record.gointernal/core/runtime/scope_phase6_secret_safety_test.gointernal/core/runtime/attempt_stream_scope_test.gointernal/core/runtime/attempt_stream.gointernal/core/auth/scope_test.go
internal/**
⚙️ CodeRabbit configuration file
internal/**: Focus on package boundaries, hidden coupling, unexported API design, concurrency safety, deterministic behavior, and whether logic belongs in this internal package.
Files:
internal/core/auxreq/scope_test.gointernal/core/execctx/views_scope_test.gointernal/stdhttp/auth/adapter_test.gointernal/archtest/scope_boundary_test.gointernal/core/runtime/secure_session.gointernal/core/auth/local_apikey_scope_test.gointernal/core/auth/errors.gointernal/core/auxreq/client.gointernal/core/runtime/executor_open_attempt.gointernal/core/execctx/views.gointernal/core/auth/local_noop_scope_test.gointernal/core/runtime/executor_secure_session_test.gointernal/core/runtime/scope_resolver_test.gointernal/core/runtime/scope_resolver.gointernal/core/auth/scope_phase6_attribution_only_test.gointernal/core/config/access_auth_local_attribution_test.gointernal/stdhttp/auth/middleware.gointernal/core/auth/scope.gointernal/core/runtime/scope_phase6_compatibility_test.gointernal/core/auth/local_noop.gointernal/core/auth/local_apikey.gointernal/core/runtime/executor_scope_test.gointernal/core/config/access_auth_validate.gointernal/core/config/access_auth_model.gointernal/core/runtime/executor_prepare_secure.gointernal/core/auth/local_apikey_record.gointernal/core/runtime/scope_phase6_secret_safety_test.gointernal/core/runtime/attempt_stream_scope_test.gointernal/stdhttp/auth/adapter.gointernal/stdhttp/auth/middleware_test.gointernal/core/runtime/attempt_stream.gointernal/core/auth/scope_test.gointernal/stdhttp/auth/scope_bridge_test.go
internal/stdhttp/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
In
internal/stdhttp/, compose the standard distribution.
Files:
internal/stdhttp/auth/adapter_test.gointernal/stdhttp/auth/middleware.gointernal/stdhttp/auth/adapter.gointernal/stdhttp/auth/middleware_test.gointernal/stdhttp/auth/scope_bridge_test.go
internal/archtest/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
In
internal/archtest/, keep architecture and hygiene gates.
Files:
internal/archtest/scope_boundary_test.go
🪛 ast-grep (0.44.0)
internal/core/auth/local_apikey_scope_test.go
[warning] 15-15: A credential is hard-coded as a string literal. Secrets stored in source code, such as passwords, API keys, and tokens, can be leaked through version control or binaries and used by internal or external malicious actors. Rotate the exposed secret and load it at runtime from a secure secret vault, a Hardware Security Module (HSM), or an environment variable if permitted by your company policy (e.g. password := os.Getenv("APP_PASSWORD")).
Context: secret := "my-api-key-value-16"
Note: [CWE-798] Use of Hard-coded Credentials.
(hardcoded-credentials-string-literal-go)
[warning] 82-82: A credential is hard-coded as a string literal. Secrets stored in source code, such as passwords, API keys, and tokens, can be leaked through version control or binaries and used by internal or external malicious actors. Rotate the exposed secret and load it at runtime from a secure secret vault, a Hardware Security Module (HSM), or an environment variable if permitted by your company policy (e.g. password := os.Getenv("APP_PASSWORD")).
Context: secret := "my-api-key-value-16"
Note: [CWE-798] Use of Hard-coded Credentials.
(hardcoded-credentials-string-literal-go)
🪛 LanguageTool
.kiro/specs/control-plane-principal-scope/tasks.md
[style] ~179-~179: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...values reach safe scope or evidence.
- Task 6.3 (boundary / attribution-only fo...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~180-~180: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...dmin-GUI code on the affected paths.
- Task 7.1 (focused verification command s...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (62)
pkg/lipsdk/scope/value.go (1)
1-35: LGTM!pkg/lipsdk/scope/value_test.go (1)
1-88: LGTM!pkg/lipsdk/scope/view.go (2)
33-61: LGTM! Clone correctly preserves nil-vs-empty semantics for slices/maps viaslices.Clone/maps.Clone, matching the isolation tests in view_test.go.
63-75: LGTM! Projection correctly copies Roles/SafeClaims so the legacyexecview.PrincipalViewcan't alias the authoritative scope, verified byTestPrincipalScopeView_PrincipalProjectionIsCopy.pkg/lipsdk/scope/view_test.go (1)
1-228: LGTM! Good coverage, including the forbidden-field-name reflection guard and nil-preservation checks.pkg/lipsdk/scope/doc.go (1)
1-14: LGTM!pkg/lipsdk/scope/context.go (1)
9-32: LGTM!pkg/lipsdk/scope/context_test.go (1)
1-72: LGTM!pkg/lipsdk/transport/httpauth/context.go (1)
7-7: LGTM!Also applies to: 20-30
pkg/lipsdk/transport/httpauth/context_scope_test.go (1)
1-35: LGTM!internal/stdhttp/auth/adapter.go (4)
141-186:bridgeScopelogic reviewed — matches precedence rules and tests.Allow path defers to
coreauth.BuildScope, treatsErrNoIdentityas legacy pass-through, and rejects any other error before execution; deny/challenge path clones+sanitizes any supplied scope and drops it silently on sanitize failure. This lines up withscope_bridge_test.gocoverage (allow/legacy/deny/challenge/unsafe cases). No further issues beyond thed.Scopeclearing note above.
318-374: Scope-to-legacy-principal projection inauthDecisionEventlooks correct.
srccorrectly prefersevidenceScope.Principal()overd.Principal, claims are key-only (values cleared) to avoid leaking claim values onto the audit path, andev.Scopeis only populated (and cloned) whenevidenceScopeis non-nil. MatchesTestPolicyProvider_allow_evidenceCarriesSafeScopeAndCompatFieldsand the unsafe-scope omission tests.
6-6: LGTM!Also applies to: 16-16
135-144: LGTM!internal/stdhttp/auth/adapter_test.go (1)
358-375: LGTM!internal/stdhttp/auth/middleware.go (2)
99-104: LGTM!
196-224: 📐 Maintainability & Code QualityNo lingering
EnsureContextPrincipalreferences remain.internal/stdhttp/auth/middleware_test.go (1)
15-15: LGTM!Also applies to: 336-397
internal/stdhttp/auth/scope_bridge_test.go (1)
1-391: LGTM!internal/archtest/scope_boundary_test.go (2)
11-44: LGTM!Substring matching against generic terms like
"oauth"/"saml"is coarse but appropriately conservative for an architecture guardrail. As per coding guidelines,internal/archtest/**/*.goshould keep architecture and hygiene gates, which this satisfies.Source: Coding guidelines
46-89: LGTM!docs/dogfood-local.md (2)
90-121: LGTM!
136-136: 📐 Maintainability & Code QualityArchived spec path is present.
.kiro/specs/control-plane-principal-scope/tasks.md (1)
173-181: LGTM!internal/core/auth/local_apikey.go (1)
9-15: LGTM!Also applies to: 29-29, 47-47, 81-129
internal/core/auth/local_apikey_record.go (1)
13-40: LGTM!Also applies to: 74-127
internal/core/auth/local_apikey_scope_test.go (1)
1-131: LGTM!internal/core/auth/local_noop.go (1)
9-9: LGTM!Also applies to: 46-66
internal/core/auth/local_noop_scope_test.go (1)
1-67: LGTM!internal/core/config/access_auth_model.go (1)
16-35: LGTM!internal/core/config/access_auth_validate.go (1)
80-91: LGTM!internal/core/config/access_auth_local_attribution_test.go (1)
33-145: LGTM!internal/core/execctx/views.go (1)
44-57: LGTM!Also applies to: 59-80
internal/core/execctx/views_scope_test.go (1)
1-109: LGTM!internal/core/runtime/scope_resolver.go (2)
45-57: LGTM!
28-43: 🎯 Functional CorrectnessRequest scope is written back into context before traffic capture reads it.
internal/core/runtime/scope_resolver_test.go (1)
1-138: LGTM!internal/core/runtime/secure_session.go (1)
29-35: LGTM!internal/core/runtime/executor_secure_session_test.go (1)
81-100: LGTM!internal/core/runtime/executor_open_attempt.go (1)
369-378: LGTM!internal/core/runtime/executor_prepare_secure.go (1)
27-27: LGTM!Also applies to: 66-72, 157-157, 241-241, 360-360
internal/core/runtime/attempt_stream.go (1)
303-303: LGTM!Moving
recvExecContext(ctx)before the recover-drain branch correctly ensures synthesized usage events during stream recovery carry the scoped context (matchesTestRuntime_usageEvidence_recoveryDrainCarriesScope), and sourcingPrincipalID/ScopefromscopeFromCtx(ctx)keeps traffic/usage emission consistent with the authoritative scope resolved upstream.Also applies to: 561-569, 769-771, 786-786
internal/core/runtime/attempt_stream_scope_test.go (1)
1-338: LGTM!Solid coverage of scope propagation into usage/traffic evidence, secret-free leg checks, synthetic local fallback, and recovery-drain scope carryover.
internal/core/runtime/executor_scope_test.go (1)
1-244: LGTM!Good coverage of trusted-scope precedence over legacy principal, legacy-principal-derived scope fallback, and multi-attempt scope sharing without disturbing recovery semantics.
internal/core/runtime/scope_phase6_compatibility_test.go (1)
1-228: LGTM!Good verification that streaming/non-streaming paths carry identical scope, optional attribution doesn't affect routing/attempt counts, and canonical CTP payload shape/fields remain unaffected by scope attachment.
internal/core/runtime/scope_phase6_secret_safety_test.go (1)
1-178: LGTM!Good coverage of secret-free session-start evidence derivation and isolation of scope copies between usage/traffic observer events.
internal/core/auxreq/client.go (1)
13-13: LGTM!Correctly preserves parent scope while marking derived origin as internal and updating
ParentTraceID; matchesTestClient_Stream_preservesParentScopeAndMarksInternalOriginand the no-scope fallback test.Also applies to: 53-63
internal/core/auxreq/scope_test.go (1)
1-85: LGTM!pkg/lipsdk/auth/decision.go (1)
3-6: LGTM!Also applies to: 35-38
pkg/lipsdk/auth/decision_scope_test.go (1)
1-49: LGTM!pkg/lipsdk/auth/events_scope_test.go (1)
1-67: LGTM!pkg/lipsdk/execview/views.go (1)
4-6: LGTM!internal/core/auth/errors.go (1)
10-20: LGTM!internal/core/auth/scope_test.go (1)
1-253: LGTM!internal/core/auth/scope_phase6_attribution_only_test.go (1)
1-78: LGTM!pkg/lipsdk/auth/events.go (1)
3-7: LGTM!Also applies to: 60-63
pkg/lipsdk/transport/httpauth/result.go (1)
7-7: LGTM!Also applies to: 33-37
pkg/lipsdk/transport/httpauth/result_scope_test.go (1)
1-45: LGTM!pkg/lipsdk/traffic/emit.go (1)
8-8: LGTM!Also applies to: 72-72
pkg/lipsdk/traffic/observe.go (1)
30-46: LGTM!Also applies to: 26-29
pkg/lipsdk/traffic/observe_scope_test.go (1)
1-135: LGTM!pkg/lipsdk/usage/observe_scope_test.go (1)
1-61: LGTM!
- BuildScope: reject trusted scopes with no principal id (ErrNoIdentity), mirroring the legacy path - adapter: forced unsafe-scope deny always reports unsafe_scope, superseding stale allow-era reason codes - config: toCore deep-copies Roles/SafeClaims/PolicyLabels so core auth records don't alias config state - SanitizeScope doc: note the credential heuristic is best-effort/non-exhaustive - context key: document the iota offset convention - tests: cover the new branches; fix attributionConverted test to call config.ValidateAuthLocalAPIKeyRecords Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
Summary
Verification