Various non-security fixes#4512
Conversation
buixor
commented
Jun 12, 2026
buixor
commented
- Deepcopy of appsectempresponse to avoid concurrent map access
- Ensure RequestChallenge can only be called from the relevant phases (inband.post_eval and on_challenge - which is inband too)
|
@buixor: There are no 'kind' label on this PR. You need a 'kind' label to generate the release automatically.
DetailsI am a bot created to help the crowdsecurity developers manage community feedback and contributions. You can check out my manifest file to understand my behavior and what I can do. If you want to use this for your project, you can check out the BirthdayResearch/oss-governance-bot repository. |
|
@buixor: There are no area labels on this PR. You can add as many areas as you see fit.
DetailsI am a bot created to help the crowdsecurity developers manage community feedback and contributions. You can check out my manifest file to understand my behavior and what I can do. If you want to use this for your project, you can check out the BirthdayResearch/oss-governance-bot repository. |
There was a problem hiding this comment.
Pull request overview
This PR addresses two correctness issues in the AppSec pipeline: (1) avoiding concurrent map access by ensuring the HTTP handler receives an isolated copy of the in-band response, and (2) constraining challenge issuance so SendChallenge() can only be used from the intended in-band hook stages.
Changes:
- Added
AppsecTempResponse.Clone()and updated the appsec runner to send a cloned response to the handler to prevent in-band/out-of-band races. - Removed
SendChallenge()from thepre_evalexpr environment and added a runtime guard to reject out-of-bandSendChallenge()calls. - Extended/adjusted tests to validate cloning behavior, hook exposure restrictions, and challenge flow behavior.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/appsec/waf_helpers.go | Removes SendChallenge from pre_eval expr env (compile-time restriction). |
| pkg/appsec/response_clone_test.go | Adds tests validating deep clone behavior and race prevention. |
| pkg/appsec/patcher.go | Updates comment around challenge runtime callee detection. |
| pkg/appsec/patcher_test.go | Adjusts test to reference SendChallenge() from post_eval (where it is exposed). |
| pkg/appsec/challenge/challenge_test.go | Fixes malformed-ticket test input to pass missing-field validation. |
| pkg/appsec/appsec.go | Adds AppsecTempResponse.Clone() and enforces in-band-only SendChallenge() at runtime. |
| pkg/acquisition/modules/appsec/appsec_test.go | Treats hook compilation/build failures as expected when expected_load_ok is false. |
| pkg/acquisition/modules/appsec/appsec_runner.go | Sends cloned in-band response to handler; skips OOB evaluation when a challenge is served. |
| pkg/acquisition/modules/appsec/appsec_hooks_test.go | Updates hook tests from pre_eval to post_eval and adds coverage for new restrictions/flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for range 1000 { | ||
| _, body := rt.GenerateResponse(sent, rt.Logger) | ||
| _, _ = json.Marshal(body) | ||
| } |
| for i := range 1000 { | ||
| _ = rt.SetChallengeHeader(state, "X-Oob", string(rune('a'+i%26))) | ||
| } |
| // A challenge was served, so the request never reaches the backend; | ||
| if state.RequireChallenge { | ||
| return | ||
| } |
