Skip to content

fix transitive access recursion - #9924

Open
k-anshul wants to merge 3 commits into
mainfrom
anshul/fix-transitive-access-recursion
Open

k-anshul wants to merge 3 commits into
mainfrom
anshul/fix-transitive-access-recursion

Conversation

@k-anshul

@k-anshul k-anshul commented Sep 23, 2026 •

Copy link
Copy Markdown
Member

Follow up for bug discovered in #9908

Checklist:

  • Covered by tests
  • Ran it and it works as intended
  • Reviewed the diff before requesting a review
  • Checked for unhandled edge cases
  • Linked the issues it closes
  • Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
  • Intend to cherry-pick into the release branch
  • I'm proud of this work!

Introduce Runtime.AnalyzeResolver and require every resolver to register an
analyzer alongside its initializer via RegisterResolver. Analysis takes user
attributes for templating but no security claims, so it cannot re-enter
security resolution. Resolvers that cannot infer security rules register
AnalysisUnsupported.
…ecursion

Report, alert and canvas reconcilers initialized the resolver with the
caller's claims to read its refs and required security rules. Resolvers that
resolve security at init (metrics, metrics_sql, api) re-expanded the same
transitive access rule, recursing until the process died.

Use Runtime.AnalyzeResolver instead, and drop InferRequiredSecurityRules from
the Resolver interface.
@k-anshul k-anshul self-assigned this Sep 23, 2026
@k-anshul k-anshul changed the title Anshul/fix transitive access recursion fix transitive access recursion Sep 23, 2026
Runtime: r.C.Runtime,
InstanceID: r.C.InstanceID,
Properties: resolverProperties(spec),
Claims: claims,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A smaller solution is to just pass claims with SkipSecurity set to true but that is a duct tape solution and prone to bugs in new reconcilers.

@k-anshul
k-anshul marked this pull request as ready for review September 24, 2026 11:21

@begelundmuller begelundmuller left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to proceed if you want to fix the issue, but see my comment below

Comment thread runtime/resolver.go
func RegisterResolverInitializer(name string, initializer ResolverInitializer) {
// RegisterResolver registers a resolver by name.
// Every resolver must provide an analyzer; use AnalysisUnsupported if it cannot infer security rules.
func RegisterResolver(name string, initializer ResolverInitializer, analyzer ResolverAnalyzer) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's alright, but feels like the separate "analyzer" concept makes things quite a bit more confusing. Did you consider any solutions that work just with the normal Resolver interface? After all, initializing a resolver is already meant to be lightweight and to facilitate e.g. validation with Validate.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants