Skip to content

Commit d8b922d

Browse files
buixorCopilotCopilot
authored
propose an alternative, cleaner configuration for appsec-config (#4397)
* propose an alternative, cleaner configuration for appsec-config * fix tests * lint * avoid hardcoded hook names * simplify struct layout * Update pkg/appsec/appsec.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Fix early-return guards to also check PostEval hooks in ProcessInBandRules and ProcessOutOfBandRules Agent-Logs-Url: https://github.com/crowdsecurity/crowdsec/sessions/d48208fb-cd17-47ce-b881-3e04d596990f Co-authored-by: buixor <990714+buixor@users.noreply.github.com> * tests * Update pkg/acquisition/modules/appsec/appsec_test.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: buixor <990714+buixor@users.noreply.github.com>
1 parent fc1677b commit d8b922d

File tree

5 files changed

+859
-188
lines changed

5 files changed

+859
-188
lines changed

‎pkg/acquisition/modules/appsec/appsec_hooks_test.go‎

Lines changed: 257 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1082,3 +1082,260 @@ func TestOnMatchRemediationHooks(t *testing.T) {
10821082

10831083
runTests(t, tests)
10841084
}
1085+
1086+
func TestAppsecPhaseScopedHooks(t *testing.T) {
1087+
tests := []appsecRuleTest{
1088+
{
1089+
name: "inband on_match: change return code (phase-scoped, no IsInBand filter needed)",
1090+
expected_load_ok: true,
1091+
inband_rules: []appsec_rule.CustomRule{
1092+
{
1093+
Name: "rule1",
1094+
Zones: []string{"ARGS"},
1095+
Variables: []string{"foo"},
1096+
Match: appsec_rule.Match{Type: "regex", Value: "^toto"},
1097+
Transform: []string{"lowercase"},
1098+
},
1099+
},
1100+
inband_on_match: []appsec.Hook{
1101+
{Apply: []string{"SetReturnCode(413)"}},
1102+
},
1103+
input_request: appsec.ParsedRequest{
1104+
RemoteAddr: "1.2.3.4",
1105+
Method: "GET",
1106+
URI: "/urllll",
1107+
Args: url.Values{"foo": []string{"toto"}},
1108+
HTTPRequest: &http.Request{Host: "example.com"},
1109+
},
1110+
output_asserts: func(events []pipeline.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) {
1111+
require.Len(t, events, 2)
1112+
require.Len(t, responses, 1)
1113+
require.Equal(t, 413, responses[0].UserHTTPResponseCode)
1114+
},
1115+
},
1116+
{
1117+
name: "inband on_match: set remediation (phase-scoped)",
1118+
expected_load_ok: true,
1119+
inband_rules: []appsec_rule.CustomRule{
1120+
{
1121+
Name: "rule1",
1122+
Zones: []string{"ARGS"},
1123+
Variables: []string{"foo"},
1124+
Match: appsec_rule.Match{Type: "regex", Value: "^toto"},
1125+
Transform: []string{"lowercase"},
1126+
},
1127+
},
1128+
inband_on_match: []appsec.Hook{
1129+
{Apply: []string{"SetRemediation('captcha')"}},
1130+
},
1131+
input_request: appsec.ParsedRequest{
1132+
RemoteAddr: "1.2.3.4",
1133+
Method: "GET",
1134+
URI: "/urllll",
1135+
Args: url.Values{"foo": []string{"toto"}},
1136+
HTTPRequest: &http.Request{Host: "example.com"},
1137+
},
1138+
output_asserts: func(events []pipeline.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) {
1139+
require.Len(t, responses, 1)
1140+
require.Equal(t, "captcha", responses[0].Action)
1141+
},
1142+
},
1143+
{
1144+
name: "inband on_match: cancel event (phase-scoped)",
1145+
expected_load_ok: true,
1146+
inband_rules: []appsec_rule.CustomRule{
1147+
{
1148+
Name: "rule1",
1149+
Zones: []string{"ARGS"},
1150+
Variables: []string{"foo"},
1151+
Match: appsec_rule.Match{Type: "regex", Value: "^toto"},
1152+
Transform: []string{"lowercase"},
1153+
},
1154+
},
1155+
inband_on_match: []appsec.Hook{
1156+
{Apply: []string{"CancelEvent()"}},
1157+
},
1158+
input_request: appsec.ParsedRequest{
1159+
RemoteAddr: "1.2.3.4",
1160+
Method: "GET",
1161+
URI: "/urllll",
1162+
Args: url.Values{"foo": []string{"toto"}},
1163+
HTTPRequest: &http.Request{Host: "example.com"},
1164+
},
1165+
output_asserts: func(events []pipeline.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) {
1166+
// CancelEvent() only cancels the LOG event, the APPSEC alert is still sent
1167+
require.Len(t, events, 1)
1168+
require.Equal(t, pipeline.APPSEC, events[0].Type)
1169+
require.Len(t, responses, 1)
1170+
},
1171+
},
1172+
{
1173+
name: "inband on_match with filter: only fires when filter matches (phase-scoped)",
1174+
expected_load_ok: true,
1175+
inband_rules: []appsec_rule.CustomRule{
1176+
{
1177+
Name: "rule1",
1178+
Zones: []string{"ARGS"},
1179+
Variables: []string{"foo"},
1180+
Match: appsec_rule.Match{Type: "regex", Value: "^toto"},
1181+
Transform: []string{"lowercase"},
1182+
},
1183+
},
1184+
inband_on_match: []appsec.Hook{
1185+
{Filter: "evt.Appsec.HasInBandMatches == true", Apply: []string{"SetReturnCode(418)"}},
1186+
},
1187+
input_request: appsec.ParsedRequest{
1188+
RemoteAddr: "1.2.3.4",
1189+
Method: "GET",
1190+
URI: "/urllll",
1191+
Args: url.Values{"foo": []string{"toto"}},
1192+
HTTPRequest: &http.Request{Host: "example.com"},
1193+
},
1194+
output_asserts: func(events []pipeline.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) {
1195+
require.Len(t, responses, 1)
1196+
require.Equal(t, 418, responses[0].UserHTTPResponseCode)
1197+
},
1198+
},
1199+
{
1200+
name: "shared + inband on_match: both execute (shared runs first)",
1201+
expected_load_ok: true,
1202+
inband_rules: []appsec_rule.CustomRule{
1203+
{
1204+
Name: "rule1",
1205+
Zones: []string{"ARGS"},
1206+
Variables: []string{"foo"},
1207+
Match: appsec_rule.Match{Type: "regex", Value: "^toto"},
1208+
Transform: []string{"lowercase"},
1209+
},
1210+
},
1211+
on_match: []appsec.Hook{
1212+
// Shared hook: runs for both phases, sets remediation
1213+
{Filter: "IsInBand == true", Apply: []string{"SetRemediation('captcha')"}},
1214+
},
1215+
inband_on_match: []appsec.Hook{
1216+
// Phase-scoped hook: overrides the return code
1217+
{Apply: []string{"SetReturnCode(418)"}},
1218+
},
1219+
input_request: appsec.ParsedRequest{
1220+
RemoteAddr: "1.2.3.4",
1221+
Method: "GET",
1222+
URI: "/urllll",
1223+
Args: url.Values{"foo": []string{"toto"}},
1224+
HTTPRequest: &http.Request{Host: "example.com"},
1225+
},
1226+
output_asserts: func(events []pipeline.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) {
1227+
require.Len(t, responses, 1)
1228+
// Shared hook set captcha, phase-scoped hook set return code 418
1229+
require.Equal(t, "captcha", responses[0].Action)
1230+
require.Equal(t, 418, responses[0].UserHTTPResponseCode)
1231+
},
1232+
},
1233+
{
1234+
name: "shared on_match break does not prevent phase-scoped hooks",
1235+
expected_load_ok: true,
1236+
DefaultRemediation: appsec.AllowRemediation,
1237+
inband_rules: []appsec_rule.CustomRule{
1238+
{
1239+
Name: "rule1",
1240+
Zones: []string{"ARGS"},
1241+
Variables: []string{"foo"},
1242+
Match: appsec_rule.Match{Type: "regex", Value: "^toto"},
1243+
Transform: []string{"lowercase"},
1244+
},
1245+
},
1246+
on_match: []appsec.Hook{
1247+
{Filter: "IsInBand == true", Apply: []string{"CancelEvent()"}, OnSuccess: "break"},
1248+
},
1249+
inband_on_match: []appsec.Hook{
1250+
{Apply: []string{"SetRemediation('captcha')", "SetReturnCode(418)"}},
1251+
},
1252+
input_request: appsec.ParsedRequest{
1253+
RemoteAddr: "1.2.3.4",
1254+
Method: "GET",
1255+
URI: "/urllll",
1256+
Args: url.Values{"foo": []string{"toto"}},
1257+
HTTPRequest: &http.Request{Host: "example.com"},
1258+
},
1259+
output_asserts: func(events []pipeline.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) {
1260+
require.Len(t, responses, 1)
1261+
// Shared hook canceled LOG event with break, APPSEC alert still sent
1262+
// Phase-scoped hooks still run (break only stops shared hook list)
1263+
require.Len(t, events, 1)
1264+
require.Equal(t, pipeline.APPSEC, events[0].Type)
1265+
require.Equal(t, "captcha", responses[0].Action)
1266+
require.Equal(t, 418, responses[0].UserHTTPResponseCode)
1267+
},
1268+
},
1269+
{
1270+
name: "outofband on_match: send alert (phase-scoped)",
1271+
expected_load_ok: true,
1272+
DefaultRemediation: appsec.AllowRemediation,
1273+
outofband_rules: []appsec_rule.CustomRule{
1274+
{
1275+
Name: "rule1",
1276+
Zones: []string{"ARGS"},
1277+
Variables: []string{"foo"},
1278+
Match: appsec_rule.Match{Type: "regex", Value: "^toto"},
1279+
Transform: []string{"lowercase"},
1280+
},
1281+
},
1282+
outofband_on_match: []appsec.Hook{
1283+
{Apply: []string{"SendAlert()"}},
1284+
},
1285+
input_request: appsec.ParsedRequest{
1286+
RemoteAddr: "1.2.3.4",
1287+
Method: "GET",
1288+
URI: "/urllll",
1289+
Args: url.Values{"foo": []string{"toto"}},
1290+
HTTPRequest: &http.Request{Host: "example.com"},
1291+
},
1292+
output_asserts: func(events []pipeline.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) {
1293+
require.Len(t, responses, 1)
1294+
require.Equal(t, appsec.AllowRemediation, appsecResponse.Action)
1295+
// outofband matched and sent an alert event
1296+
require.NotEmpty(t, events)
1297+
foundAppsecEvt := false
1298+
for _, evt := range events {
1299+
if evt.Type == pipeline.APPSEC {
1300+
foundAppsecEvt = true
1301+
}
1302+
}
1303+
require.True(t, foundAppsecEvt, "expected an APPSEC event from outofband match")
1304+
},
1305+
},
1306+
{
1307+
name: "inband on_match with break+continue: break stops inband hooks",
1308+
expected_load_ok: true,
1309+
DefaultRemediation: appsec.AllowRemediation,
1310+
inband_rules: []appsec_rule.CustomRule{
1311+
{
1312+
Name: "rule1",
1313+
Zones: []string{"ARGS"},
1314+
Variables: []string{"foo"},
1315+
Match: appsec_rule.Match{Type: "regex", Value: "^toto"},
1316+
Transform: []string{"lowercase"},
1317+
},
1318+
},
1319+
inband_on_match: []appsec.Hook{
1320+
// break requires a filter to be present and match (sets has_match flag)
1321+
{Filter: "evt.Appsec.HasInBandMatches == true", Apply: []string{"SetRemediation('captcha')", "SetReturnCode(418)"}, OnSuccess: "break"},
1322+
{Filter: "evt.Appsec.HasInBandMatches == true", Apply: []string{"SetRemediation('ban')"}}, // should not execute due to break
1323+
},
1324+
input_request: appsec.ParsedRequest{
1325+
RemoteAddr: "1.2.3.4",
1326+
Method: "GET",
1327+
URI: "/urllll",
1328+
Args: url.Values{"foo": []string{"toto"}},
1329+
HTTPRequest: &http.Request{Host: "example.com"},
1330+
},
1331+
output_asserts: func(events []pipeline.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) {
1332+
require.Len(t, responses, 1)
1333+
// First hook ran (captcha + 418), second was skipped due to break
1334+
require.Equal(t, "captcha", responses[0].Action)
1335+
require.Equal(t, 418, responses[0].UserHTTPResponseCode)
1336+
},
1337+
},
1338+
}
1339+
1340+
runTests(t, tests)
1341+
}

‎pkg/acquisition/modules/appsec/appsec_runner.go‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -220,8 +220,12 @@ func (r *AppsecRunner) processRequest(state *appsec.AppsecRequestState, request
220220
func (r *AppsecRunner) ProcessInBandRules(state *appsec.AppsecRequestState, request *appsec.ParsedRequest) error {
221221
tx := appsec.NewExtendedTransaction(r.AppsecInbandEngine, request.UUID)
222222
state.Tx = tx
223-
// Even if we have no inband rules, we might have pre-eval rules to process
224-
if len(r.AppsecRuntime.InBandRules) == 0 && len(r.AppsecRuntime.CompiledPreEval) == 0 {
223+
// Even if we have no inband rules, we might have pre-eval or post-eval rules to process
224+
if len(r.AppsecRuntime.InBandRules) == 0 &&
225+
len(r.AppsecRuntime.CommonHooks.PreEval) == 0 &&
226+
len(r.AppsecRuntime.InBandHooks.PreEval) == 0 &&
227+
len(r.AppsecRuntime.CommonHooks.PostEval) == 0 &&
228+
len(r.AppsecRuntime.InBandHooks.PostEval) == 0 {
225229
return nil
226230
}
227231
err := r.processRequest(state, request)
@@ -231,7 +235,11 @@ func (r *AppsecRunner) ProcessInBandRules(state *appsec.AppsecRequestState, requ
231235
func (r *AppsecRunner) ProcessOutOfBandRules(state *appsec.AppsecRequestState, request *appsec.ParsedRequest) error {
232236
tx := appsec.NewExtendedTransaction(r.AppsecOutbandEngine, request.UUID)
233237
state.Tx = tx
234-
if len(r.AppsecRuntime.OutOfBandRules) == 0 && len(r.AppsecRuntime.CompiledPreEval) == 0 {
238+
if len(r.AppsecRuntime.OutOfBandRules) == 0 &&
239+
len(r.AppsecRuntime.CommonHooks.PreEval) == 0 &&
240+
len(r.AppsecRuntime.OutOfBandHooks.PreEval) == 0 &&
241+
len(r.AppsecRuntime.CommonHooks.PostEval) == 0 &&
242+
len(r.AppsecRuntime.OutOfBandHooks.PostEval) == 0 {
235243
return nil
236244
}
237245
err := r.processRequest(state, request)

‎pkg/acquisition/modules/appsec/appsec_test.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,13 @@ type appsecRuleTest struct {
3232
pre_eval []appsec.Hook
3333
post_eval []appsec.Hook
3434
on_match []appsec.Hook
35+
// Phase-scoped hooks (dispatched only during the matching phase)
36+
inband_on_match []appsec.Hook
37+
inband_pre_eval []appsec.Hook
38+
inband_post_eval []appsec.Hook
39+
outofband_on_match []appsec.Hook
40+
outofband_pre_eval []appsec.Hook
41+
outofband_post_eval []appsec.Hook
3542
BouncerBlockedHTTPCode int
3643
UserBlockedHTTPCode int
3744
UserPassedHTTPCode int
@@ -115,6 +122,23 @@ func testAppSecEngine(t *testing.T, test appsecRuleTest) {
115122
DefaultPassAction: test.DefaultPassAction,
116123
}
117124

125+
// Set phase-scoped hooks if any are provided
126+
if len(test.inband_on_match) > 0 || len(test.inband_pre_eval) > 0 || len(test.inband_post_eval) > 0 {
127+
appsecCfg.InBand = &appsec.AppsecPhaseConfig{
128+
OnMatch: test.inband_on_match,
129+
PreEval: test.inband_pre_eval,
130+
PostEval: test.inband_post_eval,
131+
}
132+
}
133+
134+
if len(test.outofband_on_match) > 0 || len(test.outofband_pre_eval) > 0 || len(test.outofband_post_eval) > 0 {
135+
appsecCfg.OutOfBand = &appsec.AppsecPhaseConfig{
136+
OnMatch: test.outofband_on_match,
137+
PreEval: test.outofband_pre_eval,
138+
PostEval: test.outofband_post_eval,
139+
}
140+
}
141+
118142
hub := cwhub.Hub{}
119143
AppsecRuntime, err := appsecCfg.Build(&hub)
120144
if err != nil {

0 commit comments

Comments
 (0)