Skip to content

Commit 9caa62e

Browse files
fix: resolve auth secret file race condition using sync.Once (projectdiscovery#6976)
This commit fixes a race condition where templates start executing before the secret file's login flow completes when using -secret-file flag. Changes: - pkg/authprovider/authx/dynamic.go: Replace atomic.Bool-based synchronization with sync.Once to ensure fetch callback runs exactly once and all concurrent callers block until completion. This prevents the race where multiple goroutines would see fetching=true and return early with nil error before secrets were ready. - internal/runner/runner.go: Always prefetch secrets when AuthProvider exists, removing the PreFetchSecrets flag check. This ensures authentication completes before scanning starts regardless of flag settings. - lib/sdk_private.go: Same change as runner.go for SDK mode consistency. - pkg/authprovider/file_test.go: Add test for concurrent dynamic secret access to verify fetch happens exactly once and all workers get authenticated. - pkg/authprovider/authx/dynamic_test.go: Add concurrent fetch tests to verify proper synchronization behavior. Fixes projectdiscovery#6592 Co-authored-by: Mzack9999 <mzack9999@protonmail.com>
1 parent 35e6e94 commit 9caa62e

File tree

5 files changed

+230
-38
lines changed

5 files changed

+230
-38
lines changed

internal/runner/runner.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -687,8 +687,8 @@ func (r *Runner) RunEnumeration() error {
687687
// display execution info like version , templates used etc
688688
r.displayExecutionInfo(store)
689689

690-
// prefetch secrets if enabled
691-
if executorOpts.AuthProvider != nil && r.options.PreFetchSecrets {
690+
// prefetch secrets to ensure authentication completes before scanning starts
691+
if executorOpts.AuthProvider != nil {
692692
r.Logger.Info().Msgf("Pre-fetching secrets from authprovider[s]")
693693
if err := executorOpts.AuthProvider.PreFetchSecrets(); err != nil {
694694
return errors.Wrap(err, "could not pre-fetch secrets")

lib/sdk_private.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,8 +228,8 @@ func (e *NucleiEngine) init(ctx context.Context) error {
228228
e.executerOpts.AuthProvider = e.authprovider
229229
}
230230

231-
// prefetch secrets
232-
if e.executerOpts.AuthProvider != nil && e.opts.PreFetchSecrets {
231+
// prefetch secrets to ensure authentication completes before scanning starts
232+
if e.executerOpts.AuthProvider != nil {
233233
if err := e.executerOpts.AuthProvider.PreFetchSecrets(); err != nil {
234234
return errors.Wrap(err, "could not prefetch secrets")
235235
}

pkg/authprovider/authx/dynamic.go

Lines changed: 39 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package authx
33
import (
44
"fmt"
55
"strings"
6-
"sync/atomic"
6+
"sync"
77

88
"github.com/projectdiscovery/gologger"
99
"github.com/projectdiscovery/nuclei/v3/pkg/protocols/common/replacer"
@@ -14,6 +14,13 @@ import (
1414

1515
type LazyFetchSecret func(d *Dynamic) error
1616

17+
// fetchState holds the sync.Once and error for thread-safe fetching.
18+
// This is stored as a pointer in Dynamic so that value copies share the same state.
19+
type fetchState struct {
20+
once sync.Once
21+
err error
22+
}
23+
1724
var (
1825
_ json.Unmarshaler = &Dynamic{}
1926
)
@@ -30,9 +37,9 @@ type Dynamic struct {
3037
Input string `json:"input" yaml:"input"` // (optional) target for the dynamic secret
3138
Extracted map[string]interface{} `json:"-" yaml:"-"` // extracted values from the dynamic secret
3239
fetchCallback LazyFetchSecret `json:"-" yaml:"-"`
33-
fetched *atomic.Bool `json:"-" yaml:"-"` // atomic flag to check if the secret has been fetched
34-
fetching *atomic.Bool `json:"-" yaml:"-"` // atomic flag to prevent recursive fetch calls
35-
error error `json:"-" yaml:"-"` // error if any
40+
// fetchState is shared across value-copies of Dynamic (e.g., inside DynamicAuthStrategy).
41+
// It must be initialized via Validate() before calling Fetch().
42+
fetchState *fetchState `json:"-" yaml:"-"`
3643
}
3744

3845
func (d *Dynamic) GetDomainAndDomainRegex() ([]string, []string) {
@@ -70,8 +77,9 @@ func (d *Dynamic) UnmarshalJSON(data []byte) error {
7077

7178
// Validate validates the dynamic secret
7279
func (d *Dynamic) Validate() error {
73-
d.fetched = &atomic.Bool{}
74-
d.fetching = &atomic.Bool{}
80+
// NOTE: Validate() must not be called concurrently with Fetch()/GetStrategies().
81+
// Re-validating resets fetch state and allows re-fetching.
82+
d.fetchState = &fetchState{}
7583
if d.TemplatePath == "" {
7684
return errkit.New(" template-path is required for dynamic secret")
7785
}
@@ -181,18 +189,14 @@ func (d *Dynamic) applyValuesToSecret(secret *Secret) error {
181189
return nil
182190
}
183191

184-
// GetStrategy returns the auth strategies for the dynamic secret
192+
// GetStrategies returns the auth strategies for the dynamic secret
185193
func (d *Dynamic) GetStrategies() []AuthStrategy {
186-
if d.fetched.Load() {
187-
if d.error != nil {
188-
return nil
189-
}
190-
} else {
191-
// Try to fetch if not already fetched
192-
_ = d.Fetch(true)
193-
}
194+
// Ensure fetch has completed before returning strategies.
195+
// Fetch errors are treated as non-fatal here so a failed dynamic auth fetch
196+
// does not terminate the entire scan process.
197+
_ = d.Fetch(false)
194198

195-
if d.error != nil {
199+
if d.fetchState != nil && d.fetchState.err != nil {
196200
return nil
197201
}
198202
var strategies []AuthStrategy
@@ -208,30 +212,31 @@ func (d *Dynamic) GetStrategies() []AuthStrategy {
208212
// Fetch fetches the dynamic secret
209213
// if isFatal is true, it will stop the execution if the secret could not be fetched
210214
func (d *Dynamic) Fetch(isFatal bool) error {
211-
if d.fetched.Load() {
212-
return d.error
213-
}
214-
215-
// Try to set fetching flag atomically
216-
if !d.fetching.CompareAndSwap(false, true) {
217-
// Already fetching, return current error
218-
return d.error
215+
if d.fetchState == nil {
216+
if isFatal {
217+
gologger.Fatal().Msgf("Could not fetch dynamic secret: Validate() must be called before Fetch()")
218+
}
219+
return errkit.New("dynamic secret not validated: call Validate() before Fetch()")
219220
}
220221

221-
// We're the only one fetching, call the callback
222-
d.error = d.fetchCallback(d)
223-
224-
// Mark as fetched and clear fetching flag
225-
d.fetched.Store(true)
226-
d.fetching.Store(false)
222+
d.fetchState.once.Do(func() {
223+
if d.fetchCallback == nil {
224+
d.fetchState.err = errkit.New("dynamic secret fetch callback not set: call SetLazyFetchCallback() before Fetch()")
225+
return
226+
}
227+
d.fetchState.err = d.fetchCallback(d)
228+
})
227229

228-
if d.error != nil && isFatal {
229-
gologger.Fatal().Msgf("Could not fetch dynamic secret: %s\n", d.error)
230+
if d.fetchState.err != nil && isFatal {
231+
gologger.Fatal().Msgf("Could not fetch dynamic secret: %s\n", d.fetchState.err)
230232
}
231-
return d.error
233+
return d.fetchState.err
232234
}
233235

234236
// Error returns the error if any
235237
func (d *Dynamic) Error() error {
236-
return d.error
238+
if d.fetchState == nil {
239+
return nil
240+
}
241+
return d.fetchState.err
237242
}

pkg/authprovider/authx/dynamic_test.go

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
package authx
22

33
import (
4+
"errors"
5+
"sync"
6+
"sync/atomic"
47
"testing"
8+
"time"
59

610
"github.com/stretchr/testify/require"
711
)
@@ -123,3 +127,100 @@ func TestDynamicUnmarshalJSON(t *testing.T) {
123127
require.NoError(t, err)
124128
})
125129
}
130+
131+
func TestDynamicFetchConcurrent(t *testing.T) {
132+
t.Run("all-waiters-block-until-done", func(t *testing.T) {
133+
const numGoroutines = 10
134+
wantErr := errors.New("auth fetch failed")
135+
fetchStarted := make(chan struct{})
136+
fetchUnblock := make(chan struct{})
137+
138+
d := &Dynamic{
139+
TemplatePath: "test-template.yaml",
140+
Variables: []KV{{Key: "username", Value: "test"}},
141+
}
142+
require.NoError(t, d.Validate())
143+
d.SetLazyFetchCallback(func(_ *Dynamic) error {
144+
close(fetchStarted)
145+
<-fetchUnblock
146+
return wantErr
147+
})
148+
149+
results := make([]error, numGoroutines)
150+
var wg sync.WaitGroup
151+
wg.Add(numGoroutines)
152+
153+
for i := 0; i < numGoroutines; i++ {
154+
go func(idx int) {
155+
defer wg.Done()
156+
results[idx] = d.Fetch(false)
157+
}(i)
158+
}
159+
160+
select {
161+
case <-fetchStarted:
162+
case <-time.After(5 * time.Second):
163+
t.Fatal("fetch callback never started")
164+
}
165+
166+
done := make(chan struct{})
167+
go func() {
168+
wg.Wait()
169+
close(done)
170+
}()
171+
select {
172+
case <-done:
173+
t.Fatal("fetch callers returned before fetch completed")
174+
case <-time.After(25 * time.Millisecond):
175+
}
176+
177+
close(fetchUnblock)
178+
select {
179+
case <-done:
180+
case <-time.After(5 * time.Second):
181+
t.Fatal("fetch callers did not complete in time")
182+
}
183+
184+
for _, err := range results {
185+
require.ErrorIs(t, err, wantErr)
186+
}
187+
})
188+
189+
t.Run("fetch-callback-runs-once", func(t *testing.T) {
190+
const numGoroutines = 20
191+
var callCount atomic.Int32
192+
errs := make(chan error, numGoroutines)
193+
barrier := make(chan struct{})
194+
195+
d := &Dynamic{
196+
TemplatePath: "test-template.yaml",
197+
Variables: []KV{{Key: "username", Value: "test"}},
198+
}
199+
require.NoError(t, d.Validate())
200+
d.SetLazyFetchCallback(func(dynamic *Dynamic) error {
201+
callCount.Add(1)
202+
time.Sleep(20 * time.Millisecond)
203+
dynamic.Extracted = map[string]interface{}{"token": "secret-token"}
204+
return nil
205+
})
206+
207+
var wg sync.WaitGroup
208+
wg.Add(numGoroutines)
209+
for i := 0; i < numGoroutines; i++ {
210+
go func() {
211+
defer wg.Done()
212+
<-barrier
213+
errs <- d.Fetch(false)
214+
}()
215+
}
216+
close(barrier)
217+
wg.Wait()
218+
close(errs)
219+
220+
for err := range errs {
221+
require.NoError(t, err)
222+
}
223+
224+
require.Equal(t, int32(1), callCount.Load(), "fetch callback must be called exactly once")
225+
})
226+
}

pkg/authprovider/file_test.go

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
package authprovider
2+
3+
import (
4+
"fmt"
5+
"net/http"
6+
"os"
7+
"path/filepath"
8+
"sync"
9+
"sync/atomic"
10+
"testing"
11+
"time"
12+
13+
"github.com/projectdiscovery/nuclei/v3/pkg/authprovider/authx"
14+
"github.com/stretchr/testify/require"
15+
)
16+
17+
func TestFileAuthProviderDynamicSecretConcurrentAccess(t *testing.T) {
18+
secretFile := filepath.Join(t.TempDir(), "secret.yaml")
19+
secretData := []byte(`id: test-auth
20+
info:
21+
name: test
22+
author: test
23+
severity: info
24+
dynamic:
25+
- template: auth-template.yaml
26+
variables:
27+
- key: username
28+
value: test
29+
type: Header
30+
domains:
31+
- example.com
32+
headers:
33+
- key: Authorization
34+
value: "Bearer {{token}}"
35+
`)
36+
require.NoError(t, os.WriteFile(secretFile, secretData, 0o600))
37+
38+
var fetchCalls atomic.Int32
39+
provider, err := NewFileAuthProvider(secretFile, func(dynamic *authx.Dynamic) error {
40+
fetchCalls.Add(1)
41+
time.Sleep(75 * time.Millisecond)
42+
dynamic.Extracted = map[string]interface{}{"token": "session-token"}
43+
return nil
44+
})
45+
require.NoError(t, err)
46+
47+
const workers = 20
48+
barrier := make(chan struct{})
49+
errs := make(chan error, workers)
50+
var wg sync.WaitGroup
51+
wg.Add(workers)
52+
53+
for i := 0; i < workers; i++ {
54+
go func() {
55+
defer wg.Done()
56+
<-barrier
57+
58+
strategies := provider.LookupAddr("example.com")
59+
if len(strategies) == 0 {
60+
errs <- fmt.Errorf("no auth strategies found")
61+
return
62+
}
63+
64+
req, reqErr := http.NewRequest(http.MethodGet, "https://example.com", nil)
65+
if reqErr != nil {
66+
errs <- reqErr
67+
return
68+
}
69+
for _, strategy := range strategies {
70+
strategy.Apply(req)
71+
}
72+
if got := req.Header.Get("Authorization"); got != "Bearer session-token" {
73+
errs <- fmt.Errorf("expected Authorization header to be set, got %q", got)
74+
}
75+
}()
76+
}
77+
78+
close(barrier)
79+
wg.Wait()
80+
close(errs)
81+
82+
for gotErr := range errs {
83+
require.NoError(t, gotErr)
84+
}
85+
require.Equal(t, int32(1), fetchCalls.Load(), "dynamic secret fetch should execute once")
86+
}

0 commit comments

Comments
 (0)