Repository navigation
fix(http/raw): persist QueryAuthStrategy params in ApplyAuthStrategy - #7410
Conversation
The QueryAuthStrategy case in raw.Request.ApplyAuthStrategy parses r.FullURL into a local 'parsed', appends auth params to parsed.Params, then returns without writing back to r.FullURL. Every other branch in the same switch mutates r.Headers, and the standalone QueryAuthStrategy .Apply / .ApplyOnRR in pkg/authprovider/authx/query_auth.go both write back to req.URL.RawQuery. Only the raw-request branch was missing the write-back, so raw templates matching a 'query' auth secret silently drop the auth params. Adds r.FullURL = parsed.String() after the loop. New unit test fails on dev and passes with the fix.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughApplyAuthStrategy's QueryAuthStrategy branch now writes the updated URL (with appended query auth params) back into Request.FullURL. A set of unit tests was added to verify query param persistence, preservation of existing params, invalid-URL behavior, nil-strategy no-op, cookies, headers, bearer token, and basic auth handling. ChangesQuery Auth Parameter Persistence + Tests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
nuclei 3.9.0 Created-by: HarmonybrewBot Commit-by: HarmonybrewBot Merged-by: HarmonybrewBot Description: Created by `brew bump` --- Created with `brew bump-formula-pr`.<details> <summary>release notes</summary> <pre>## What's Changed ### 🎉 New Features * Added support for protocol redirects by @Mzack9999 in projectdiscovery/nuclei#7296 * Added impacket integration by @Mzack9999 in projectdiscovery/nuclei#7356 * Added wmi, tsch, scmr, and dcom helper modules for JS by @dwisiswant0 in projectdiscovery/nuclei#7388 * Added mirroring of -config, -report-config, and -dashboard flags for SDK callers by @ShubhamRasal in projectdiscovery/nuclei#7393 ### 🐞 Bug Fixes * Fixed DNS variables resolving issue by @Mzack9999 in projectdiscovery/nuclei#7379 * Fixed expressions to prefer exact placeholders over expressions by @dwisiswant0 in projectdiscovery/nuclei#7397 * Fixed an issue in JS where runtimes that outlive the interrupt grace period are not abandoned, by @Mzack9999 in projectdiscovery/nuclei#7378 * Fixed an issue in reporting/jira where status-not was not matched against the transition target status name by @knakul853 in projectdiscovery/nuclei#7361 * Fixed runner to write unsigned-templates warning (WRN) to stderr by @ChrisJr404 in projectdiscovery/nuclei#7371 * Fixed `InternalEvent` access serialization in interactsh by @dwisiswant0 in projectdiscovery/nuclei#7322 * Fixed workflow to ensure context is properly propagated to child and race steps by @dwisiswant0 in projectdiscovery/nuclei#7383 * Fixed reporting/gitlab by bumping client-go to v1.9.1 and widening IDs to int64 by @ShubhamRasal in projectdiscovery/nuclei#7398 * Fixed memogen to skip context correctly and auto-push scope to upstream by @Mzack9999 in projectdiscovery/nuclei#7419 * Fixed a slice-bounds panic in telnetmini's ParseNTLMResponse when handling truncated NTLM challenges by @tejgokani in projectdiscovery/nuclei#7425 * Fixed SMBv1 probing in JS to occur after SMB2 negotiation by @dwisiswant0 in projectdiscovery/nuclei#7430 * Fixed templates to reject unknown fields when loading JSON templates by @tejgokani in projectdiscovery/nuclei#7453 * Fixed persistence of QueryAuthStrategy parameters in ApplyAuthStrategy by @akhilesharora in projectdiscovery/nuclei#7410 * Fixed loader to warn when templates are excluded by `.nuclei-ignore` tags by @dogancanbakir in projectdiscovery/nuclei#7452 ### Other Changes * Splitted exact matches and pattern handling in catalog for improved performance by @mikhail5555 in projectdiscovery/nuclei#7340 * Updated README_PT-BR.md to reflect current project description and features by @gugacyber in projectdiscovery/nuclei#7405 * Fixed broken cloud scanning link by @dogancanbakir in projectdiscovery/nuclei#7413 * Bumped pdsec modules + govaluate/aurora migration by @dogancanbakir in projectdiscovery/nuclei#7414 * Dropped vulnerable docker/docker dependency by @dogancanbakir in projectdiscovery/nuclei#7447 * Used path-aware filesystem containment checks by @dwisiswant0 in projectdiscovery/nuclei#7420 * Centralized runtime session handling by @dwisiswant0 in projectdiscovery/nuclei#7449 ## New Contributors * @ChrisJr404 made their first contribution in projectdiscovery/nuclei#7371 * @gugacyber made their first contribution in projectdiscovery/nuclei#7405 * @akhilesharora made their first contribution in projectdiscovery/nuclei#7410 * @tejgokani made their first contribution in projectdiscovery/nuclei#7425 **Full Changelog**: https://github.com/projectdiscovery/nuclei/compare/v3.8.0...v3.9.0</pre> <p>View the full release notes at <a href="https://github.com/projectdiscovery/nuclei/releases/tag/v3.9.0">https://github.com/projectdiscovery/nuclei/releases/tag/v3.9.0</a>.</p> </details> <hr> See merge request: Harmonybrew/homebrew-core!11229
Proposed changes
Closes #7409
Request.ApplyAuthStrategyinpkg/protocols/http/raw/raw.goparsesr.FullURLinto a localparsed, appends theQueryAuthStrategyparams toparsed.Params, then returns without writing the result back. Every other branch of the switch (CookiesAuthStrategy,HeadersAuthStrategy,BearerTokenAuthStrategy,BasicAuthStrategy) mutatesr.Headers. The standaloneQueryAuthStrategy.ApplyandApplyOnRRinpkg/authprovider/authx/query_auth.goboth write back toreq.URL.RawQuery. Only the raw-request path was missing the write-back, so raw templates that match aqueryauth secret silently drop the params and the request goes out unauthenticated.Adding
r.FullURL = parsed.String()after the params loop fixes it. New unit test inraw_test.gofails ondevand passes with the fix.Checklist
Summary by CodeRabbit
Bug Fixes
Tests