From 5ef37da8f4b6ce80e43c3d928e5c04cd27a98e68 Mon Sep 17 00:00:00 2001 From: Dwi Siswanto Date: Tue, 2 Sep 2025 18:27:11 +0700 Subject: [PATCH 1/2] fix(runner): handle multiple resps in `GetLazyAuthFetchCallback` the callback logic does not properly accumulate results from multiple responses Signed-off-by: Dwi Siswanto --- internal/runner/lazy.go | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/internal/runner/lazy.go b/internal/runner/lazy.go index 1202620fe0..f94ab10ba6 100644 --- a/internal/runner/lazy.go +++ b/internal/runner/lazy.go @@ -105,13 +105,13 @@ func GetLazyAuthFetchCallback(opts *AuthLazyFetchOptions) authx.LazyFetchSecret var finalErr error ctx.OnResult = func(e *output.InternalWrappedEvent) { if e == nil { - finalErr = fmt.Errorf("no result found for template: %s", d.TemplatePath) return } + if !e.HasOperatorResult() { - finalErr = fmt.Errorf("no result found for template: %s", d.TemplatePath) return } + // dynamic values for k, v := range e.OperatorsResult.DynamicValues { // Iterate through all the values and choose the @@ -123,31 +123,37 @@ func GetLazyAuthFetchCallback(opts *AuthLazyFetchOptions) authx.LazyFetchSecret } } } + // named extractors for k, v := range e.OperatorsResult.Extracts { if len(v) > 0 { - data[k] = v[0] - } - } - if len(data) == 0 { - if e.OperatorsResult.Matched { - finalErr = fmt.Errorf("match found but no (dynamic/extracted) values found for template: %s", d.TemplatePath) - } else { - finalErr = fmt.Errorf("no match or (dynamic/extracted) values found for template: %s", d.TemplatePath) + // NOTE(dwisiswant0): Only set if we don't already have a + // value -- or -- if the new value is non-empty. + if _, exists := data[k]; !exists || data[k] == "" { + data[k] = v[0] + } } } + // log result of template in result file/screen _ = writer.WriteResult(e, opts.ExecOpts.Output, opts.ExecOpts.Progress, opts.ExecOpts.IssuesClient) } + _, err := tmpl.Executer.ExecuteWithResults(ctx) if err != nil { finalErr = err } + + if len(data) == 0 && finalErr == nil { + finalErr = fmt.Errorf("no extracted values found for template: %s", d.TemplatePath) + } + // store extracted result in auth context d.Extracted = data if finalErr != nil && opts.OnError != nil { opts.OnError(finalErr) } + return finalErr } } From e7968de4314983424c4cfe9cffcd78dbc64a3faf Mon Sep 17 00:00:00 2001 From: Dwi Siswanto Date: Thu, 4 Sep 2025 18:30:44 +0700 Subject: [PATCH 2/2] fix(authx): prevent deadlock in dynamic auth fetching MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolve deadlock that occurs when dynamic auth templates trigger recursive auth requests during execution. RCA: 1. `GetStrategies()` calls `Fetch()` to retrieve auth creds. 2. `Fetch()` executes auth template via cb. 3. template exec triggers HTTP requests requiring auth. 4. recursive calls `GetStrategies()` → `Fetch()` cause deadlock on mutex. notable changes: * add `fetching` flag to `Dynamic` struct to track fetch-in-progress state. * modify `GetStrategies()` to return empty strategies if already fetching. * update `Fetch()` method with proper recursive call prevention. * use mutex-protected flag reads to ensure thread safety. * refactor `GetStrategies()` with local function for code reuse. this prevents infinite recursion during auth template execution while maintaining proper sync and err handling. fixes goroutine deadlocks in auth system when using dynamic secrets with templates that require auth. Signed-off-by: Dwi Siswanto --- pkg/authprovider/authx/dynamic.go | 65 +++++++++++++++++++++++++------ 1 file changed, 53 insertions(+), 12 deletions(-) diff --git a/pkg/authprovider/authx/dynamic.go b/pkg/authprovider/authx/dynamic.go index cff03147d5..eac47f708b 100644 --- a/pkg/authprovider/authx/dynamic.go +++ b/pkg/authprovider/authx/dynamic.go @@ -32,6 +32,7 @@ type Dynamic struct { fetchCallback LazyFetchSecret `json:"-" yaml:"-"` m *sync.Mutex `json:"-" yaml:"-"` // mutex for lazy fetch fetched bool `json:"-" yaml:"-"` // flag to check if the secret has been fetched + fetching bool `json:"-" yaml:"-"` // flag to check if we're currently fetching (prevents recursion) error error `json:"-" yaml:"-"` // error if any } @@ -184,20 +185,44 @@ func (d *Dynamic) applyValuesToSecret(secret *Secret) error { // GetStrategy returns the auth strategies for the dynamic secret func (d *Dynamic) GetStrategies() []AuthStrategy { - if !d.fetched { - _ = d.Fetch(true) + getStrategies := func() []AuthStrategy { + var strategies []AuthStrategy + if d.Secret != nil { + strategies = append(strategies, d.GetStrategy()) + } + + for _, secret := range d.Secrets { + strategies = append(strategies, secret.GetStrategy()) + } + + return strategies } - if d.error != nil { - return nil + + d.m.Lock() + isFetched := d.fetched + isFetching := d.fetching + d.m.Unlock() + + if isFetched { + if d.error != nil { + return nil + } + + return getStrategies() } - var strategies []AuthStrategy - if d.Secret != nil { - strategies = append(strategies, d.GetStrategy()) + + if isFetching { + // NOTE(dwisiswant0): Bail out w/ empty here, or we will yeet into + // recursion hell. See line 242-245. + return nil } - for _, secret := range d.Secrets { - strategies = append(strategies, secret.GetStrategy()) + + _ = d.Fetch(true) + if d.error != nil { + return nil } - return strategies + + return getStrategies() } // Fetch fetches the dynamic secret @@ -205,13 +230,29 @@ func (d *Dynamic) GetStrategies() []AuthStrategy { func (d *Dynamic) Fetch(isFatal bool) error { d.m.Lock() defer d.m.Unlock() + if d.fetched { + return d.error + } + + if d.fetching { return nil } + + d.fetching = true + defer func() { + d.fetching = false + }() + d.error = d.fetchCallback(d) - if d.error != nil && isFatal { - gologger.Fatal().Msgf("Could not fetch dynamic secret: %s\n", d.error) + if d.error != nil { + if isFatal { + gologger.Fatal().Msgf("Could not fetch dynamic secret: %s\n", d.error) + } + } else { + d.fetched = true } + return d.error }