From b01ffdab1eec8cd635d88744db2f069bcdceef2f Mon Sep 17 00:00:00 2001 From: Martin Dobrev Date: Fri, 7 Aug 2026 15:37:48 +0100 Subject: [PATCH 1/4] feat: Add per-rule suppression, completing the shadow mode loop Shadow mode names the rule that would have blocked. Naming it is half an answer: without a way to say "that one is wrong for this form", the operator chooses between promoting with the false positives and not promoting at all. That is the choice that stops WAFs being turned on. A suppression says a detection does not count for a scope. It hooks into execute_defense_node, which every mechanism returns through, so it applies uniformly rather than needing support in each one. The safety property that shapes the design: a mechanism reports one aggregate score for all its flags, so there is no per-flag score to subtract. A node is therefore neutralised only when *every* one of its flags was suppressed. One surviving flag means something nobody suppressed still fired, and that verdict stands. Verified live: with kw:viagra suppressed, a viagra payload goes 403 -> 200, a casino payload stays 403, and viagra+casino stays 403. Scopes accumulate -- global, vhost and endpoint all apply -- rather than the narrower replacing the broader. That is the opposite of config_resolver's deep merge, deliberately: an endpoint entry silently dropping the global ones is the wrong default for a control that makes the WAF do less. A global "*" is refused. It would be a WAF that is running and does nothing, which is too easy to reach by a single typo. Suppressions are logged when applied, bounded at 200 since the list is walked on every defense node, and never expire from cache -- an expiring suppression would silently start blocking traffic an operator deliberately allowed. UI: a Suppressions page under Security, and a Suppress action on each detection in the shadow view that carries the flag and vhost across. The recorded flag is profile-prefixed (legacy:kw:viagra) while a suppression matches what the mechanism emits (kw:viagra), so the prefix is stripped on the way. The shortcut only appears when every recorded decision came from one vhost -- otherwise the prefilled scope would be a guess, and a suppression on the wrong vhost is a hole rather than an annoyance. RBAC: admin and operator create/read/delete, viewer read. Verified live. Gates: 73 unit specs (16 new), config contracts hold, contract check 12 endpoints, integration 42 passed / 2 known gaps / 0 failed, typecheck 0 errors, lint 0 errors / 59 warnings. --- admin-ui/src/App.tsx | 2 + admin-ui/src/api/client.ts | 44 +++ admin-ui/src/api/generated.ts | 102 +++++++ admin-ui/src/components/layout/Sidebar.tsx | 2 + admin-ui/src/pages/security/Suppressions.tsx | 280 +++++++++++++++++++ admin-ui/src/pages/shadow/ShadowMode.tsx | 57 +++- docs/API_HANDLERS.md | 27 ++ docs/openapi.yaml | 59 ++++ openresty/conf/nginx.conf | 1 + openresty/lua/admin_api.lua | 11 + openresty/lua/api_handlers/suppressions.lua | 205 ++++++++++++++ openresty/lua/defense_profile_executor.lua | 15 + openresty/lua/rbac.lua | 17 ++ openresty/lua/redis_sync.lua | 41 +++ openresty/lua/suppressions.lua | 163 +++++++++++ openresty/spec/suppressions_spec.lua | 159 +++++++++++ 16 files changed, 1182 insertions(+), 3 deletions(-) create mode 100644 admin-ui/src/pages/security/Suppressions.tsx create mode 100644 openresty/lua/api_handlers/suppressions.lua create mode 100644 openresty/lua/suppressions.lua create mode 100644 openresty/spec/suppressions_spec.lua diff --git a/admin-ui/src/App.tsx b/admin-ui/src/App.tsx index 7f76210..47afcc3 100644 --- a/admin-ui/src/App.tsx +++ b/admin-ui/src/App.tsx @@ -28,6 +28,7 @@ import AttackSignatureEditor from '@/pages/security/AttackSignatureEditor' import BehavioralAnalytics from '@/pages/analytics/BehavioralAnalytics' import ClusterStatus from '@/pages/cluster/ClusterStatus' import ShadowMode from '@/pages/shadow/ShadowMode' +import Suppressions from '@/pages/security/Suppressions' import { About } from '@/pages/About' import { Users } from '@/pages/admin/Users' import { AuthProviders } from '@/pages/admin/AuthProviders' @@ -89,6 +90,7 @@ export default function App() { } /> } /> } /> + } /> } /> } /> } /> diff --git a/admin-ui/src/api/client.ts b/admin-ui/src/api/client.ts index 11538f8..1d127bd 100644 --- a/admin-ui/src/api/client.ts +++ b/admin-ui/src/api/client.ts @@ -1504,3 +1504,47 @@ export const shadowApi = { clear: () => request<{ cleared: boolean }>('/shadow/decisions', { method: 'DELETE' }), } + +// --------------------------------------------------------------------------- +// Rule suppressions +// +// The counterpart to shadow mode: shadow names the rule that would have +// blocked, a suppression says that rule is wrong for this scope. +// --------------------------------------------------------------------------- + +export type SuppressionScope = 'global' | 'vhost' | 'endpoint' + +export interface Suppression { + id: string + scope_type: SuppressionScope + scope_id?: string + flag: string + reason?: string + created_at?: number + created_by?: string +} + +export interface NewSuppression { + flag: string + scope_type?: SuppressionScope + scope_id?: string + reason?: string +} + +export const suppressionsApi = { + list: () => + request<{ suppressions: Suppression[]; count: number; max?: number }>('/suppressions'), + + create: (body: NewSuppression) => + request<{ suppression: Suppression; created: boolean }>('/suppressions', { + method: 'POST', + body: JSON.stringify(body), + }), + + remove: (id: string) => + request<{ deleted: boolean; id: string }>(`/suppressions/${encodeURIComponent(id)}`, { + method: 'DELETE', + }), + + clear: () => request<{ cleared: boolean }>('/suppressions', { method: 'DELETE' }), +} diff --git a/admin-ui/src/api/generated.ts b/admin-ui/src/api/generated.ts index 6c6323f..8b10ae2 100644 --- a/admin-ui/src/api/generated.ts +++ b/admin-ui/src/api/generated.ts @@ -195,6 +195,95 @@ export interface paths { patch?: never; trace?: never; }; + "/suppressions": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** Detections that no longer count, and where */ + get: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Every suppression currently in force */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": { + suppressions: components["schemas"]["Suppression"][]; + count: number; + /** @description Ceiling on entries; this list is walked on every defense node. */ + max?: number; + }; + }; + }; + }; + }; + put?: never; + /** Stop a detection counting for one scope */ + post: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Created */ + 201: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": { + suppression?: components["schemas"]["Suppression"]; + created?: boolean; + }; + }; + }; + /** @description Invalid input. Includes a global "*", which would disable all detection and is refused deliberately. */ + 400: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + }; + }; + /** Remove every suppression */ + delete: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Cleared */ + 200: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + }; + }; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/shadow/summary": { parameters: { query?: never; @@ -489,6 +578,19 @@ export interface components { high_event_rate?: number; }; }; + Suppression: { + /** @description Digest of scope and flag, so adding the same one twice is idempotent. */ + id: string; + /** @enum {string} */ + scope_type: "global" | "vhost" | "endpoint"; + /** @description Absent for global scope; the vhost or endpoint id otherwise. */ + scope_id?: string; + /** @description Exact detection flag, or a trailing "*" to cover a family (kw:*). */ + flag: string; + reason?: string; + created_at?: number; + created_by?: string; + }; ShadowDecision: { /** @description Unix time the decision was made */ ts: number; diff --git a/admin-ui/src/components/layout/Sidebar.tsx b/admin-ui/src/components/layout/Sidebar.tsx index d3f6e8a..7f73f10 100644 --- a/admin-ui/src/components/layout/Sidebar.tsx +++ b/admin-ui/src/components/layout/Sidebar.tsx @@ -24,6 +24,7 @@ import { Server, Fingerprint, EyeOff, + BellOff, MessageSquare, Workflow, Target, @@ -52,6 +53,7 @@ const navigation = [ name: 'Security', children: [ { name: 'Shadow Mode', href: '/security/shadow', icon: EyeOff }, + { name: 'Suppressions', href: '/security/suppressions', icon: BellOff }, { name: 'Form Timing', href: '/security/timing', icon: Clock }, { name: 'Defense Profiles', href: '/security/defense-profiles', icon: Workflow }, { name: 'Attack Signatures', href: '/security/attack-signatures', icon: Target }, diff --git a/admin-ui/src/pages/security/Suppressions.tsx b/admin-ui/src/pages/security/Suppressions.tsx new file mode 100644 index 0000000..7f13bfb --- /dev/null +++ b/admin-ui/src/pages/security/Suppressions.tsx @@ -0,0 +1,280 @@ +import { useState } from 'react' +import { useSearchParams } from 'react-router-dom' +import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query' +import { suppressionsApi, vhostsApi, endpointsApi } from '@/api/client' +import type { Suppression, SuppressionScope } from '@/api/client' +import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/components/ui/card' +import { Badge } from '@/components/ui/badge' +import { Button } from '@/components/ui/button' +import { Input } from '@/components/ui/input' +import { Label } from '@/components/ui/label' +import { Alert, AlertDescription } from '@/components/ui/alert' +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from '@/components/ui/select' +import { + Table, + TableBody, + TableCell, + TableHead, + TableHeader, + TableRow, +} from '@/components/ui/table' +import { useToast } from '@/components/ui/use-toast' +import { BellOff, Trash2, Info, Plus } from 'lucide-react' + +function formatWhen(ts?: number): string { + if (!ts) return '-' + return new Date(ts * 1000).toLocaleString() +} + +function scopeLabel(s: Suppression): string { + if (s.scope_type === 'global') return 'Everywhere' + return `${s.scope_type}: ${s.scope_id}` +} + +export default function Suppressions() { + const { toast } = useToast() + const queryClient = useQueryClient() + + // Arriving from Shadow Mode's "suppress" action, with the detection and the + // vhost it fired on already chosen. Landing on an empty form and being asked + // to retype a flag you were just looking at is how a good idea gets abandoned. + const [searchParams] = useSearchParams() + const [flag, setFlag] = useState(searchParams.get('flag') ?? '') + const [scopeType, setScopeType] = useState( + (searchParams.get('scope_type') as SuppressionScope | null) ?? 'vhost' + ) + const [scopeId, setScopeId] = useState(searchParams.get('scope_id') ?? '') + const [reason, setReason] = useState('') + + const { data, isLoading } = useQuery({ + queryKey: ['suppressions'], + queryFn: suppressionsApi.list, + }) + + const { data: vhostsData } = useQuery({ queryKey: ['vhosts'], queryFn: vhostsApi.list }) + const { data: endpointsData } = useQuery({ + queryKey: ['endpoints'], + queryFn: () => endpointsApi.list(), + }) + + // Both list endpoints wrap their array, and the items come back untyped. + // Only id and name are needed to populate the scope picker. + type ScopeOption = { id: string; name?: string } + const rawVhosts = (vhostsData as { vhosts?: ScopeOption[] } | undefined)?.vhosts + const vhosts = Array.isArray(rawVhosts) ? rawVhosts : [] + const rawEndpoints = (endpointsData as { endpoints?: ScopeOption[] } | undefined)?.endpoints + const endpoints = Array.isArray(rawEndpoints) ? rawEndpoints : [] + + const createMutation = useMutation({ + mutationFn: suppressionsApi.create, + onSuccess: (res) => { + toast({ + title: res.created ? 'Suppression added' : 'Already suppressed', + description: `${res.suppression.flag} no longer counts for ${scopeLabel(res.suppression)}.`, + }) + setFlag('') + setReason('') + queryClient.invalidateQueries({ queryKey: ['suppressions'] }) + }, + onError: (err: Error) => + toast({ title: 'Could not add suppression', description: err.message, variant: 'destructive' }), + }) + + const removeMutation = useMutation({ + mutationFn: suppressionsApi.remove, + onSuccess: () => { + toast({ title: 'Suppression removed', description: 'That detection counts again.' }) + queryClient.invalidateQueries({ queryKey: ['suppressions'] }) + }, + onError: (err: Error) => + toast({ title: 'Could not remove', description: err.message, variant: 'destructive' }), + }) + + const scopeOptions = scopeType === 'vhost' ? vhosts : endpoints + const needsScopeId = scopeType !== 'global' + const canSubmit = flag.trim().length > 0 && (!needsScopeId || scopeId.length > 0) + + return ( +
+
+

+ + Rule Suppressions +

+

+ Stop a detection counting where it is wrong, without turning the endpoint off. +

+
+ + + + + A suppression removes one detection from the result. A request that also triggered + something you did not suppress is still blocked — so suppressing{' '} + kw:viagra will not let spam through that also + matched kw:casino. Use a trailing{' '} + * to cover a family, e.g.{' '} + kw:*. + + + + + + Add a suppression + + The flag is the detection name shown in Shadow Mode, e.g.{' '} + kw:viagra or{' '} + fp_flag:suspicious-bot. + + + +
+
+ + setFlag(e.target.value)} + placeholder="kw:viagra" + /> +
+
+ + +
+
+ + +
+
+ + setReason(e.target.value)} + placeholder="Pharmacy client, legitimate term" + /> +
+
+
+ +
+ + + + + + + In force{' '} + {data?.count !== undefined && ( + + ({data.count} + {data.max ? ` of ${data.max}` : ''}) + + )} + + + Every one of these is a detection that no longer contributes to a block. + + + + {isLoading ? ( +

Loading...

+ ) : data?.suppressions?.length ? ( + + + + Detection + Scope + Reason + Added + + + + + {data.suppressions.map((s) => ( + + + {s.flag} + + + + {scopeLabel(s)} + + + + {s.reason || '-'} + + + {formatWhen(s.created_at)} + {s.created_by ? ` by ${s.created_by}` : ''} + + + + + + ))} + +
+ ) : ( +

+ Nothing suppressed. Every detection counts. +

+ )} + + +
+ ) +} diff --git a/admin-ui/src/pages/shadow/ShadowMode.tsx b/admin-ui/src/pages/shadow/ShadowMode.tsx index 7199ee7..b632219 100644 --- a/admin-ui/src/pages/shadow/ShadowMode.tsx +++ b/admin-ui/src/pages/shadow/ShadowMode.tsx @@ -1,4 +1,5 @@ import { useState } from 'react' +import { Link } from 'react-router-dom' import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query' import { shadowApi } from '@/api/client' import type { ShadowCount, ShadowImpact, ShadowScope } from '@/api/client' @@ -33,6 +34,7 @@ import { Trash2, ArrowUpCircle, Fingerprint, + BellOff, } from 'lucide-react' function formatWhen(ts: number): string { @@ -44,11 +46,30 @@ function formatWhen(ts: number): string { return `${Math.floor(secs / 86400)}d ago` } +/** + * Recorded flags are profile-prefixed (`legacy:kw:viagra`) because a decision can + * come from any profile, but a suppression matches the flag the mechanism itself + * emits (`kw:viagra`). Strip the profile segment so the suppression form is + * prefilled with something that will actually match. + */ +function detectionFlag(recorded: string): string { + const firstColon = recorded.indexOf(':') + return firstColon === -1 ? recorded : recorded.slice(firstColon + 1) +} + /** * A count bar. The relative width is what makes a list of flags readable at a * glance -- "which rule is responsible for most of this" is the whole question. */ -function CountBars({ items, empty }: { items: ShadowCount[]; empty: string }) { +function CountBars({ + items, + empty, + suppressScope, +}: { + items: ShadowCount[] + empty: string + suppressScope?: string +}) { if (!items?.length) { return

{empty}

} @@ -59,7 +80,25 @@ function CountBars({ items, empty }: { items: ShadowCount[]; empty: string }) {
{item.name} - {item.count} + + {item.count} + {suppressScope && ( + + )} +
0 + // The summary spans every scope, so a flag in it cannot always be attributed + // to one vhost. Offer the suppress shortcut only when every recorded decision + // came from the same vhost -- otherwise the prefilled scope would be a guess, + // and a suppression applied to the wrong vhost is a hole, not an annoyance. + const scopeVhosts = new Set((summary?.scopes ?? []).map((s) => s.vhost_id)) + const singleVhost = scopeVhosts.size === 1 ? [...scopeVhosts][0] : undefined + return (
@@ -227,6 +273,7 @@ export default function ShadowMode() { @@ -397,7 +444,11 @@ export default function ShadowMode() { {!!pending?.top_flags?.length && (

Mostly from

- +
)} {!!pending?.affected_endpoints?.length && ( diff --git a/docs/API_HANDLERS.md b/docs/API_HANDLERS.md index 4fc48be..13c3608 100644 --- a/docs/API_HANDLERS.md +++ b/docs/API_HANDLERS.md @@ -335,6 +335,32 @@ local ok, err = utils.validate_required(data, {"field1", "field2"}) --- +### Rule Suppressions (`api_handlers/suppressions.lua`) + +The counterpart to shadow mode. Shadow mode names the rule that would have +blocked; a suppression is how an operator says that rule is wrong *here*, +without turning the endpoint off. + +| Method | Endpoint | Description | +|--------|----------|-------------| +| GET | /suppressions | Everything currently in force | +| POST | /suppressions | Stop a detection counting for one scope | +| DELETE | /suppressions | Remove all suppressions | +| DELETE | /suppressions/{id} | Remove one | + +Applied in `defense_profile_executor.execute_defense_node`, which every +mechanism returns through. Suppressed flags are removed from the node result; +a node is neutralised only when *every* one of its flags was suppressed. A +mechanism reports one aggregate score for all its flags, so there is no +per-flag score to subtract — which is why a node that also fired for an +unsuppressed reason keeps its verdict. `kw:*` covers a family; a global `*` +is refused, because that is a WAF that is on and does nothing. + +Scopes accumulate (global + vhost + endpoint all apply) rather than the +narrower replacing the broader, which is the opposite of the deep-merge +`config_resolver` uses — deliberately, since a narrow entry silently dropping +the global ones is the wrong default for a safety control. + ### Shadow Mode (`api_handlers/shadow.lua`) What monitoring mode *would* have blocked, so a rule set can be proven before it @@ -372,6 +398,7 @@ Each endpoint requires specific permissions. See [RBAC Guide](RBAC.md) for full | security | read, update | | slack | read, update, test | | shadow | read, promote, delete | +| suppressions | create, read, delete | --- diff --git a/docs/openapi.yaml b/docs/openapi.yaml index ad98122..9434aac 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -113,6 +113,24 @@ components: high_event_count: { type: integer } high_event_rate: { type: integer } + Suppression: + type: object + required: [id, scope_type, flag] + properties: + id: + type: string + description: Digest of scope and flag, so adding the same one twice is idempotent. + scope_type: { type: string, enum: [global, vhost, endpoint] } + scope_id: + type: string + description: Absent for global scope; the vhost or endpoint id otherwise. + flag: + type: string + description: Exact detection flag, or a trailing "*" to cover a family (kw:*). + reason: { type: string } + created_at: { type: integer } + created_by: { type: string } + ShadowDecision: type: object required: [ts, vhost_id, endpoint_id, score] @@ -264,6 +282,47 @@ paths: emitted_events: { type: array, items: { type: string } } unavailable_events: { type: array, items: { type: string } } + /suppressions: + get: + summary: Detections that no longer count, and where + responses: + "200": + description: Every suppression currently in force + content: + application/json: + schema: + type: object + required: [suppressions, count] + properties: + suppressions: + type: array + items: { $ref: "#/components/schemas/Suppression" } + count: { type: integer } + max: + type: integer + description: Ceiling on entries; this list is walked on every defense node. + post: + summary: Stop a detection counting for one scope + responses: + "201": + description: Created + content: + application/json: + schema: + type: object + properties: + suppression: { $ref: "#/components/schemas/Suppression" } + created: { type: boolean } + "400": + description: > + Invalid input. Includes a global "*", which would disable all + detection and is refused deliberately. + delete: + summary: Remove every suppression + responses: + "200": + description: Cleared + /shadow/summary: get: summary: What monitoring mode would have blocked diff --git a/openresty/conf/nginx.conf b/openresty/conf/nginx.conf index 45892ec..5c557f5 100644 --- a/openresty/conf/nginx.conf +++ b/openresty/conf/nginx.conf @@ -136,6 +136,7 @@ http { lua_shared_dict waf_timing 1m; # Timing token encryption keys lua_shared_dict coordinator_cache 1m; # Instance coordinator state lua_shared_dict shadow_cache 10m; # Shadow-mode would-block buffer (flushed to Redis by a timer) + lua_shared_dict suppression_cache 1m; # Rule suppressions, read on every defense node # Initialize modules on worker start init_worker_by_lua_block { diff --git a/openresty/lua/admin_api.lua b/openresty/lua/admin_api.lua index 5aa4e31..15fb377 100644 --- a/openresty/lua/admin_api.lua +++ b/openresty/lua/admin_api.lua @@ -20,6 +20,7 @@ local config_handler = require "api_handlers.config" local webhooks_handler = require "api_handlers.webhooks" local slack_handler = require "api_handlers.slack" local shadow_handler = require "api_handlers.shadow" +local suppressions_handler = require "api_handlers.suppressions" local geoip_handler = require "api_handlers.geoip" local reputation_handler = require "api_handlers.reputation" local bulk_handler = require "api_handlers.bulk" @@ -70,6 +71,7 @@ register_handlers(config_handler) register_handlers(webhooks_handler) register_handlers(slack_handler) register_handlers(shadow_handler) +register_handlers(suppressions_handler) register_handlers(geoip_handler) register_handlers(reputation_handler) register_handlers(bulk_handler) @@ -201,6 +203,15 @@ function _M.handle_request() end end + -- Check for parameterized suppression routes: DELETE /suppressions/{id} + local suppression_id = path:match("^/suppressions/([a-fA-F0-9]+)$") + if suppression_id then + local handler = suppressions_handler.resource_handlers[method] + if handler then + return handler(suppression_id) + end + end + -- Check for parameterized vhost routes: /vhosts/{id} or /vhosts/{id}/action local vhost_id, vhost_action = path:match("^/vhosts/([a-zA-Z0-9_-]+)/?([a-z]*)$") diff --git a/openresty/lua/api_handlers/suppressions.lua b/openresty/lua/api_handlers/suppressions.lua new file mode 100644 index 0000000..60678c0 --- /dev/null +++ b/openresty/lua/api_handlers/suppressions.lua @@ -0,0 +1,205 @@ +-- api_handlers/suppressions.lua +-- Rule suppressions: "this detection does not count for this scope". +-- +-- The counterpart to shadow mode. Shadow mode names the rule that would have +-- blocked; this is how an operator says that rule is wrong here, without +-- turning the whole endpoint off. + +local _M = {} + +local utils = require "api_handlers.utils" +local cjson = require "cjson.safe" +local redis_sync = require "redis_sync" +local suppressions = require "suppressions" + +local REDIS_KEY = "waf:suppressions" + +local VALID_SCOPES = { global = true, vhost = true, endpoint = true } + +-- An empty Lua table encodes as a JSON object; the UI calls .map on these. +local function as_array(t) + if type(t) ~= "table" then + return setmetatable({}, cjson.array_mt) + end + return setmetatable(t, cjson.array_mt) +end + +--- A stable id from the scope and flag, so the same suppression added twice is +--- the same entry rather than a duplicate that has to be deleted twice. +local function suppression_id(scope_type, scope_id, flag) + return ngx.md5(scope_type .. "|" .. (scope_id or "") .. "|" .. flag) +end + +local function read_all(red) + local raw, err = red:hgetall(REDIS_KEY) + if not raw then return nil, err end + + local out = {} + if type(raw) == "table" then + for i = 1, #raw, 2 do + local decoded = cjson.decode(raw[i + 1]) + if decoded then + decoded.id = decoded.id or raw[i] + out[#out + 1] = decoded + end + end + end + table.sort(out, function(a, b) + return (a.created_at or 0) > (b.created_at or 0) + end) + return out +end + +_M.handlers = {} + +-- GET /suppressions - everything currently in force +_M.handlers["GET:/suppressions"] = function() + local red, err = utils.get_redis() + if not red then + return utils.error_response("Redis connection failed: " .. (err or "unknown")) + end + + local entries, read_err = read_all(red) + utils.close_redis(red) + if not entries then + return utils.error_response("Failed to read suppressions: " .. (read_err or "unknown")) + end + + return utils.json_response({ + suppressions = as_array(entries), + count = #entries, + max = suppressions.get_max(), + }) +end + +-- POST /suppressions - stop a detection counting for one scope +_M.handlers["POST:/suppressions"] = function() + local body, err = utils.get_json_body() + if not body then + return utils.error_response(err or "Invalid JSON body", 400) + end + + local valid, missing = utils.validate_required(body, { "flag" }) + if not valid then + return utils.error_response("Missing required field: " .. missing, 400) + end + + local flag = tostring(body.flag) + if #flag == 0 or #flag > 120 then + return utils.error_response("flag must be 1-120 characters", 400) + end + + local scope_type = body.scope_type or "global" + if not VALID_SCOPES[scope_type] then + return utils.error_response("scope_type must be one of: global, vhost, endpoint", 400) + end + + local scope_id = body.scope_id + if scope_type ~= "global" then + if not scope_id or scope_id == "" then + return utils.error_response("scope_id is required when scope_type is " .. scope_type, 400) + end + scope_id = tostring(scope_id) + else + scope_id = nil + end + + -- A bare "*" would suppress every detection everywhere, which is a WAF that + -- is on but does nothing -- the failure mode this feature must not enable by + -- a single typo. Turning the vhost off is the honest way to express that. + if flag == "*" and scope_type == "global" then + return utils.error_response( + "Refusing a global '*' suppression: that disables all detection. " .. + "Scope it to a vhost or endpoint, or set the vhost mode to passthrough.", 400) + end + + local red, conn_err = utils.get_redis() + if not red then + return utils.error_response("Redis connection failed: " .. (conn_err or "unknown")) + end + + local existing = read_all(red) + local id = suppression_id(scope_type, scope_id, flag) + local already = false + for _, entry in ipairs(existing or {}) do + if entry.id == id then already = true break end + end + + -- Bounded: this list is walked on every defense node of every request. + if not already and existing and #existing >= suppressions.get_max() then + utils.close_redis(red) + return utils.error_response( + "Suppression limit reached (" .. suppressions.get_max() .. "). " .. + "Delete unused entries, or use a trailing '*' to cover a family in one rule.", 400) + end + + local entry = { + id = id, + scope_type = scope_type, + scope_id = scope_id, + flag = flag, + reason = body.reason and tostring(body.reason):sub(1, 500) or nil, + created_at = ngx.time(), + created_by = ngx.ctx.admin_user and ngx.ctx.admin_user.username or "unknown", + } + + local encoded = cjson.encode(entry) + local ok, set_err = red:hset(REDIS_KEY, id, encoded) + utils.close_redis(red) + if not ok then + return utils.error_response("Failed to store suppression: " .. (set_err or "unknown")) + end + + -- Push it to this pod's workers now; other pods pick it up on their timer. + redis_sync.sync_now() + + ngx.log(ngx.WARN, "SUPPRESSION_ADDED: flag=", flag, " scope=", scope_type, + ":", tostring(scope_id), " by=", entry.created_by) + + return utils.json_response({ + suppression = entry, + created = not already, + }, already and 200 or 201) +end + +-- DELETE /suppressions - remove every suppression at once +_M.handlers["DELETE:/suppressions"] = function() + local red, err = utils.get_redis() + if not red then + return utils.error_response("Redis connection failed: " .. (err or "unknown")) + end + + red:del(REDIS_KEY) + utils.close_redis(red) + redis_sync.sync_now() + + ngx.log(ngx.WARN, "SUPPRESSIONS_CLEARED by=", + ngx.ctx.admin_user and ngx.ctx.admin_user.username or "unknown") + + return utils.json_response({ cleared = true }) +end + +-- Parametric: DELETE /suppressions/{id} +_M.resource_handlers = {} + +_M.resource_handlers["DELETE"] = function(id) + local red, err = utils.get_redis() + if not red then + return utils.error_response("Redis connection failed: " .. (err or "unknown")) + end + + local removed = red:hdel(REDIS_KEY, id) + utils.close_redis(red) + + if not removed or removed == 0 then + return utils.error_response("Suppression not found: " .. tostring(id), 404) + end + + redis_sync.sync_now() + ngx.log(ngx.WARN, "SUPPRESSION_REMOVED: id=", id, " by=", + ngx.ctx.admin_user and ngx.ctx.admin_user.username or "unknown") + + return utils.json_response({ deleted = true, id = id }) +end + +return _M diff --git a/openresty/lua/defense_profile_executor.lua b/openresty/lua/defense_profile_executor.lua index 3dea3ea..13058e0 100644 --- a/openresty/lua/defense_profile_executor.lua +++ b/openresty/lua/defense_profile_executor.lua @@ -423,6 +423,21 @@ local function execute_defense_node(node, request_context) return _M.result_score(0, {"defense_error:" .. defense_name}, {error = result}) end + -- Every mechanism returns through here, which is why suppression hooks in at + -- this point rather than in each mechanism. A suppression an operator added + -- after seeing a false positive in the shadow view takes effect here. + local suppressions = require "suppressions" + local _, removed = suppressions.apply(result, request_context.vhost_id, + request_context.endpoint_id) + if removed then + -- Logged, not silent: a detection that stops counting is exactly the + -- thing an operator needs to be able to find later when asking why + -- something got through. + ngx.log(ngx.INFO, "SUPPRESSED: ", defense_name, " flags=", + table.concat(removed, ","), " vhost=", tostring(request_context.vhost_id), + " endpoint=", tostring(request_context.endpoint_id)) + end + return result end diff --git a/openresty/lua/rbac.lua b/openresty/lua/rbac.lua index e1475dd..537147b 100644 --- a/openresty/lua/rbac.lua +++ b/openresty/lua/rbac.lua @@ -38,6 +38,7 @@ local DEFAULT_ROLES = { webhooks = {"read", "update", "test"}, slack = {"read", "update", "test", "reset"}, shadow = {"read", "promote", "delete"}, + suppressions = {"create", "read", "delete"}, geoip = {"read", "update", "reload"}, reputation = {"read", "update"}, timing = {"read", "update"}, @@ -70,6 +71,7 @@ local DEFAULT_ROLES = { webhooks = {"read"}, slack = {"read"}, shadow = {"read"}, + suppressions = {"create", "read", "delete"}, geoip = {"read"}, reputation = {"read"}, timing = {"read"}, @@ -99,6 +101,7 @@ local DEFAULT_ROLES = { webhooks = {"read"}, slack = {"read"}, shadow = {"read"}, + suppressions = {"read"}, geoip = {"read"}, reputation = {"read"}, timing = {"read"}, @@ -192,6 +195,9 @@ local ENDPOINT_PERMISSIONS = { ["GET:/shadow/impact"] = {resource = "shadow", action = "read"}, ["POST:/shadow/promote"] = {resource = "shadow", action = "promote"}, ["DELETE:/shadow/decisions"] = {resource = "shadow", action = "delete"}, + ["GET:/suppressions"] = {resource = "suppressions", action = "read"}, + ["POST:/suppressions"] = {resource = "suppressions", action = "create"}, + ["DELETE:/suppressions"] = {resource = "suppressions", action = "delete"}, -- Slack notifications ["GET:/slack/config"] = {resource = "slack", action = "read"}, @@ -297,6 +303,12 @@ local ENDPOINT_PERMISSIONS = { -- Parametric endpoint permissions (for /endpoints/{id}, /vhosts/{id}, etc.) local PARAMETRIC_PERMISSIONS = { + -- Suppressions. Not vhost-scoped: the id is a digest of the scope and flag, + -- not a vhost, so a scoped check would have nothing to match against. The + -- scope is enforced when the suppression is created, not when it is deleted. + suppressions = { + ["DELETE"] = {resource = "suppressions", action = "delete"}, + }, -- Endpoints endpoints = { ["GET"] = {resource = "endpoints", action = "read", scoped = true}, @@ -530,6 +542,11 @@ function _M.get_endpoint_permission(method, path) return PARAMETRIC_PERMISSIONS.endpoints[handler_key], endpoint_id, "endpoint" end + local suppression_id = path:match("^/suppressions/([a-fA-F0-9]+)$") + if suppression_id then + return PARAMETRIC_PERMISSIONS.suppressions[method], suppression_id, "suppression" + end + local vhost_id = path:match("^/vhosts/([a-zA-Z0-9_-]+)/?([a-z]*)$") if vhost_id then local action = path:match("^/vhosts/[a-zA-Z0-9_-]+/([a-z]+)$") diff --git a/openresty/lua/redis_sync.lua b/openresty/lua/redis_sync.lua index d0ee1e6..9367374 100644 --- a/openresty/lua/redis_sync.lua +++ b/openresty/lua/redis_sync.lua @@ -104,6 +104,7 @@ local KEYS = { thresholds = "waf:config:thresholds", routing = "waf:config:routing", ip_whitelist = "waf:whitelist:ips", + suppressions = "waf:suppressions", -- Endpoint configuration keys endpoint_index = "waf:endpoints:index", endpoint_config_prefix = "waf:endpoints:config:", @@ -284,6 +285,45 @@ local function sync_thresholds(red) end end +-- Sync rule suppressions +-- Read on every defense node of every request, so it lands in its own dict and +-- is stored pre-decoded as one JSON blob rather than as N keys to look up. +local function sync_suppressions(red) + local suppression_cache = ngx.shared.suppression_cache + if not suppression_cache then + return + end + + local entries, err = red:hgetall(KEYS.suppressions) + if not entries then + ngx.log(ngx.WARN, "Failed to get suppressions: ", err) + return + end + + local cjson = require "cjson.safe" + local list = {} + if type(entries) == "table" then + for i = 1, #entries, 2 do + local decoded = cjson.decode(entries[i + 1]) + if decoded then + decoded.id = decoded.id or entries[i] + list[#list + 1] = decoded + end + end + end + + local encoded = cjson.encode(list) + if encoded then + -- No TTL: an expiring suppression would silently start blocking traffic + -- an operator deliberately allowed. It is replaced on every sync instead. + local ok, set_err = suppression_cache:set("suppressions", encoded) + if not ok then + ngx.log(ngx.ERR, "Failed to cache suppressions: ", set_err) + end + end + ngx.log(ngx.DEBUG, "Synced ", #list, " suppressions") +end + -- Sync routing configuration local function sync_routing(red) local config, err = red:hgetall(KEYS.routing) @@ -1130,6 +1170,7 @@ local function do_sync() sync_thresholds(red) sync_routing(red) sync_ip_whitelist(red) + sync_suppressions(red) -- Sync endpoint configurations sync_endpoints(red) diff --git a/openresty/lua/suppressions.lua b/openresty/lua/suppressions.lua new file mode 100644 index 0000000..1d54b54 --- /dev/null +++ b/openresty/lua/suppressions.lua @@ -0,0 +1,163 @@ +--[[ + Rule suppression + ================ + Shadow mode answers "what would blocking have rejected?". It names the rule + responsible -- kw:viagra, fp_flag:suspicious-bot -- but naming it is only half + an answer. Without a way to say "that one is wrong for this form", the + operator's only choices are to promote and accept the false positives, or not + to promote at all. That is the choice that stops WAFs being turned on. + + A suppression says: for this scope, this detection does not count. + + Applied at the node result + -------------------------- + Every defense mechanism returns through execute_defense_node, so that is the + single place this has to hook into. Suppressed flags are removed from the + node's result; if a node's flags are *entirely* suppressed, the node is + neutralised -- score zeroed, block cleared. + + The honest limitation: a mechanism reports one aggregate score for all its + flags, so there is no per-flag score to subtract. Suppressing one of several + flags on a node therefore removes the flag but leaves the score, and the + remaining detections still stand on their own. That is the conservative + direction -- a partially-suppressed node can still block, and cannot be used + to silently defeat a rule that is also matching something else. + + Scope precedence is additive, not overriding. A global suppression, a vhost + one and an endpoint one all apply to a request in that endpoint; the narrower + scope adds to the broader rather than replacing it. Deep-merge semantics + (as config_resolver uses) would let an endpoint entry silently drop the + global ones, which is the wrong default for a safety control. +]] + +local cjson = require "cjson.safe" + +local _M = {} + +local CACHE_KEY = "suppressions" + +-- Bounded on purpose: this list is read on every defense node of every request. +local MAX_SUPPRESSIONS = 200 + +local function cache() + return ngx.shared.suppression_cache +end + +--- Every suppression currently in force, as stored by redis_sync. +-- @return array of {id, scope_type, scope_id, flag, reason, created_at, created_by} +function _M.get_all() + local dict = cache() + if not dict then return {} end + + local raw = dict:get(CACHE_KEY) + if not raw then return {} end + + local decoded = cjson.decode(raw) + if type(decoded) ~= "table" then return {} end + return decoded +end + +--- Does `flag` match `pattern`? +-- Exact match, or a trailing `*` covering a whole family: `kw:*` suppresses +-- every keyword detection without needing one entry per word. Deliberately not +-- a Lua pattern -- an operator typing a rule name should not have to know that +-- `-` and `.` mean something, and a bad pattern here fails open. +function _M.matches(flag, pattern) + if type(flag) ~= "string" or type(pattern) ~= "string" then return false end + if pattern == flag then return true end + + local prefix = pattern:match("^(.-)%*$") + if prefix then + return prefix == "" or flag:sub(1, #prefix) == prefix + end + return false +end + +--- Is this suppression in force for this scope? +local function applies_to_scope(entry, vhost_id, endpoint_id) + local scope_type = entry.scope_type or "global" + if scope_type == "global" then + return true + elseif scope_type == "vhost" then + return entry.scope_id == vhost_id + elseif scope_type == "endpoint" then + return entry.scope_id == endpoint_id + end + return false +end + +--- The flag patterns in force for one scope. +function _M.active_patterns(vhost_id, endpoint_id) + local patterns = {} + for _, entry in ipairs(_M.get_all()) do + if entry.flag and applies_to_scope(entry, vhost_id, endpoint_id) then + patterns[#patterns + 1] = entry.flag + end + end + return patterns +end + +--- Apply suppressions to one defense node's result. +-- Returns the result (mutated in place) and the list of flags removed, so the +-- caller can report what was suppressed rather than the detection just vanishing. +function _M.apply(result, vhost_id, endpoint_id) + if type(result) ~= "table" then return result, nil end + + local original = result.flags + if type(original) ~= "table" or #original == 0 then + return result, nil + end + + local patterns = _M.active_patterns(vhost_id, endpoint_id) + if #patterns == 0 then + return result, nil + end + + local kept, removed = {}, {} + for _, flag in ipairs(original) do + local suppressed = false + for _, pattern in ipairs(patterns) do + if _M.matches(flag, pattern) then + suppressed = true + break + end + end + if suppressed then + removed[#removed + 1] = flag + else + kept[#kept + 1] = flag + end + end + + if #removed == 0 then + return result, nil + end + + result.flags = kept + + -- Only a node whose every detection was suppressed loses its verdict. One + -- surviving flag means something the operator did not suppress still fired, + -- and that finding is left intact. + if #kept == 0 then + result.score = 0 + result.blocked = false + result.block_reason = nil + result.details = result.details or {} + result.details.suppressed = removed + else + result.details = result.details or {} + result.details.suppressed = removed + end + + return result, removed +end + +function _M.get_cache_key() + return CACHE_KEY +end + +function _M.get_max() + return MAX_SUPPRESSIONS +end + +return _M diff --git a/openresty/spec/suppressions_spec.lua b/openresty/spec/suppressions_spec.lua new file mode 100644 index 0000000..1469fb3 --- /dev/null +++ b/openresty/spec/suppressions_spec.lua @@ -0,0 +1,159 @@ +--[[ + Suppression is a safety control that makes the WAF do less, so its failure + modes run in the dangerous direction. Two properties carry the weight: + + * A suppression must not reach beyond its scope. An entry added for one + vhost silently applying to another is a hole opened by accident. + + * Suppressing one detection must not clear a node that also fired for a + reason nobody suppressed. Mechanisms report one aggregate score for all + their flags, so there is no per-flag score to subtract -- which means the + only safe rule is that a node keeps its verdict while any flag survives. +]] +local helper = require "spec_helper" + +describe("suppressions", function() + local suppressions + + local function install(entries) + local cjson = require "cjson.safe" + ngx.shared.suppression_cache:set("suppressions", cjson.encode(entries or {})) + end + + before_each(function() + helper.install_ngx() + helper.stub_external_modules() + package.loaded["suppressions"] = nil + suppressions = require "suppressions" + install({}) + end) + + describe("pattern matching", function() + it("matches an exact flag", function() + assert.is_true(suppressions.matches("kw:viagra", "kw:viagra")) + assert.is_false(suppressions.matches("kw:viagra", "kw:casino")) + end) + + it("matches a family with a trailing star", function() + assert.is_true(suppressions.matches("kw:viagra", "kw:*")) + assert.is_true(suppressions.matches("kw:casino", "kw:*")) + assert.is_false(suppressions.matches("fp_flag:bot", "kw:*")) + end) + + it("treats regex metacharacters as literals", function() + -- An operator typing a rule name should not have to know that "-" and + -- "." mean something. A flag containing them must match itself, and a + -- pattern using them must not match anything else. + assert.is_true(suppressions.matches("kw:crypto-investment", "kw:crypto-investment")) + assert.is_false(suppressions.matches("kw:viagra", "kw:.")) + assert.is_false(suppressions.matches("kwXviagra", "kw.viagra")) + end) + + it("rejects non-string input rather than erroring", function() + assert.is_false(suppressions.matches(nil, "kw:*")) + assert.is_false(suppressions.matches("kw:viagra", nil)) + assert.is_false(suppressions.matches(42, "kw:*")) + end) + end) + + describe("scoping", function() + it("applies a global suppression everywhere", function() + install({ { flag = "kw:viagra", scope_type = "global" } }) + local patterns = suppressions.active_patterns("any-vhost", "any-endpoint") + assert.same({ "kw:viagra" }, patterns) + end) + + it("does not leak a vhost suppression to another vhost", function() + install({ { flag = "kw:viagra", scope_type = "vhost", scope_id = "pharmacy" } }) + assert.same({ "kw:viagra" }, suppressions.active_patterns("pharmacy", "contact")) + assert.same({}, suppressions.active_patterns("other-site", "contact")) + end) + + it("does not leak an endpoint suppression to another endpoint", function() + install({ { flag = "honeypot:filled", scope_type = "endpoint", scope_id = "signup" } }) + assert.same({ "honeypot:filled" }, suppressions.active_patterns("v", "signup")) + assert.same({}, suppressions.active_patterns("v", "contact")) + end) + + it("accumulates scopes rather than the narrower one replacing the broader", function() + install({ + { flag = "kw:viagra", scope_type = "global" }, + { flag = "kw:casino", scope_type = "vhost", scope_id = "shop" }, + { flag = "honeypot:filled", scope_type = "endpoint", scope_id = "signup" }, + }) + local patterns = suppressions.active_patterns("shop", "signup") + table.sort(patterns) + assert.same({ "honeypot:filled", "kw:casino", "kw:viagra" }, patterns) + end) + + it("ignores an unknown scope type instead of applying it", function() + install({ { flag = "kw:viagra", scope_type = "everywhere-ish" } }) + assert.same({}, suppressions.active_patterns("v", "e")) + end) + end) + + describe("applying to a defense result", function() + it("neutralises a node whose every flag was suppressed", function() + install({ { flag = "kw:viagra", scope_type = "global" } }) + local result = { score = 0, blocked = true, block_reason = "keyword_blocked", + flags = { "kw:viagra" } } + local out, removed = suppressions.apply(result, "v", "e") + assert.is_false(out.blocked) + assert.equals(0, out.score) + assert.is_nil(out.block_reason) + assert.same({ "kw:viagra" }, removed) + end) + + it("leaves a node that also fired for an unsuppressed reason", function() + -- The property that stops suppression being a back door: casino was + -- not suppressed, so the block stands. + install({ { flag = "kw:viagra", scope_type = "global" } }) + local result = { score = 35, blocked = true, block_reason = "keyword_blocked", + flags = { "kw:viagra", "kw:casino" } } + local out = suppressions.apply(result, "v", "e") + assert.is_true(out.blocked) + assert.equals(35, out.score) + assert.same({ "kw:casino" }, out.flags) + end) + + it("records what it removed, so a detection never just vanishes", function() + install({ { flag = "kw:*", scope_type = "global" } }) + local result = { score = 20, blocked = false, flags = { "kw:viagra", "fp_flag:bot" } } + local out, removed = suppressions.apply(result, "v", "e") + assert.same({ "kw:viagra" }, removed) + assert.same({ "kw:viagra" }, out.details.suppressed) + assert.same({ "fp_flag:bot" }, out.flags) + end) + + it("is a no-op when nothing matches", function() + install({ { flag = "kw:viagra", scope_type = "global" } }) + local result = { score = 40, blocked = true, flags = { "fp_flag:bot" } } + local out, removed = suppressions.apply(result, "v", "e") + assert.is_true(out.blocked) + assert.equals(40, out.score) + assert.is_nil(removed) + end) + + it("is a no-op when no suppressions are configured", function() + local result = { score = 40, blocked = true, flags = { "kw:viagra" } } + local out, removed = suppressions.apply(result, "v", "e") + assert.is_true(out.blocked) + assert.is_nil(removed) + end) + + it("leaves a flagless result alone", function() + install({ { flag = "kw:*", scope_type = "global" } }) + local result = { score = 50, blocked = true, flags = {} } + local out = suppressions.apply(result, "v", "e") + assert.is_true(out.blocked, "a node with no flags has nothing to suppress") + assert.equals(50, out.score) + end) + + it("survives junk in the cache rather than failing open or erroring", function() + ngx.shared.suppression_cache:set("suppressions", "not json at all") + local result = { score = 40, blocked = true, flags = { "kw:viagra" } } + local out = suppressions.apply(result, "v", "e") + assert.is_true(out.blocked, "unreadable config must not suppress anything") + end) + end) +end) From 8649d68581888a7d078f24750a416e00e22d18e4 Mon Sep 17 00:00:00 2001 From: Martin Dobrev Date: Fri, 7 Aug 2026 15:37:48 +0100 Subject: [PATCH 2/4] fix: Declare shadow_cache and 10 env vars in the Helm ConfigMap The chart does not mount the image's nginx.conf, it ships its own copy in a ConfigMap. Anything added to one and not the other fails silently, and only on Kubernetes -- never in the compose stack, which is where testing happens. Two live consequences. Shadow mode does not work on Kubernetes at all. lua_shared_dict shadow_cache was never added to the chart, so ngx.shared.shadow_cache is nil, and shadow_recorder guards on that and returns early. No crash, no log, just an empty sample forever. That is my regression, merged in PR #39. HAPROXY_TIMEOUT is set by the openresty deployment but was not declared, so os.getenv returns nil and the value an operator puts in values.yaml is ignored. This is the same variable whose regression was fixed in PR #36 -- the fix works under compose and has been dead on Kubernetes throughout. Also declares the eight other variables nginx.conf knows and the chart did not: REDIS_TLS, WAF_ADMIN_COOKIE, WAF_ADMIN_URL, WAF_ALLOW_INTERNAL_URLS, WAF_DISABLE_SSRF_PROTECTION, WAF_LOCAL_AUTH, WAF_LOG_HMAC_KEY, WAF_SESSION_TTL and WAF_TRUSTED_PROXIES. None are set by the chart today so nothing is currently broken by them, but without the declaration an operator cannot make them work via extraEnv either. WAF_TRUSTED_PROXIES is the one that matters: it drives F01 client-IP extraction, in exactly the ingress topology where getting the client IP right is hardest. scripts/check-helm-drift.py compares the two files and fails on any difference in either direction, and runs in CI. Verified it catches drift by deleting a dict and confirming a non-zero exit. --- .github/workflows/quality.yml | 7 ++ .../templates/openresty-configmap.yaml | 12 +++ scripts/check-helm-drift.py | 100 ++++++++++++++++++ 3 files changed, 119 insertions(+) create mode 100755 scripts/check-helm-drift.py diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index 6b7ffb0..a401626 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -107,6 +107,13 @@ jobs: -v "$PWD/openresty/spec:/app/spec:ro" \ forms-waf-lua-test:ci --verbose /app/spec + # The chart ships its own copy of nginx.conf, so an env declaration or a + # shared dict added in one place and not the other fails silently and only + # on Kubernetes. Shadow mode shipped missing its dict; HAPROXY_TIMEOUT was + # set by the deployment but undeclared, so Lua never saw it. + - name: Helm / nginx.conf drift + run: python3 scripts/check-helm-drift.py + # Guards the seam between config_resolver and the defense mechanisms, where # six shipped features were silently inert. - name: Config contract diff --git a/helm/forms-waf/templates/openresty-configmap.yaml b/helm/forms-waf/templates/openresty-configmap.yaml index 081e3a3..abaf2b2 100644 --- a/helm/forms-waf/templates/openresty-configmap.yaml +++ b/helm/forms-waf/templates/openresty-configmap.yaml @@ -24,6 +24,16 @@ data: env WAF_ADMIN_PASSWORD; env HOSTNAME; env WAF_USE_LEADER_ELECTION; + env HAPROXY_TIMEOUT; + env REDIS_TLS; + env WAF_ADMIN_COOKIE; + env WAF_ADMIN_URL; + env WAF_ALLOW_INTERNAL_URLS; + env WAF_DISABLE_SSRF_PROTECTION; + env WAF_LOCAL_AUTH; + env WAF_LOG_HMAC_KEY; + env WAF_SESSION_TTL; + env WAF_TRUSTED_PROXIES; worker_processes auto; error_log /var/log/nginx/error.log warn; @@ -92,6 +102,8 @@ data: lua_shared_dict waf_metrics 10m; lua_shared_dict waf_timing 1m; lua_shared_dict coordinator_cache 1m; + lua_shared_dict shadow_cache 10m; + lua_shared_dict suppression_cache 1m; # Initialize modules on worker start init_worker_by_lua_block { diff --git a/scripts/check-helm-drift.py b/scripts/check-helm-drift.py new file mode 100755 index 0000000..8bf3117 --- /dev/null +++ b/scripts/check-helm-drift.py @@ -0,0 +1,100 @@ +#!/usr/bin/env python3 +""" +Guard the seam between openresty/conf/nginx.conf and the Helm chart's own copy +of it in templates/openresty-configmap.yaml. + +Why this exists +--------------- +The chart does not mount the image's nginx.conf; it ships its own in a +ConfigMap. So every `env NAME;` declaration and every `lua_shared_dict` has to +be added in two places, and forgetting the second one fails silently and only +on Kubernetes: + + * A missing `env NAME;` makes os.getenv("NAME") return nil inside Lua, so a + value an operator sets in values.yaml is quietly ignored. HAPROXY_TIMEOUT + was in exactly this state -- set by the deployment, undeclared in the + ConfigMap, therefore dead. + + * A missing lua_shared_dict makes ngx.shared.NAME nil. Modules guard on that + and return early, so the feature does not crash, it just never does + anything. Shadow mode shipped in this state. + +Both are the "silently inert" class this codebase has been bitten by before, +and neither shows up in the compose stack, which is where testing happens. + +Exit codes: 0 in sync, 1 drifted. +""" + +import re +import sys +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +NGINX_CONF = ROOT / "openresty" / "conf" / "nginx.conf" +HELM_CONFIGMAP = ROOT / "helm" / "forms-waf" / "templates" / "openresty-configmap.yaml" + +ENV_RE = re.compile(r"^\s*env\s+([A-Z_][A-Z0-9_]*)\s*;", re.MULTILINE) +DICT_RE = re.compile(r"^\s*lua_shared_dict\s+([a-z_][a-z0-9_]*)\s", re.MULTILINE) + + +def extract(path, pattern): + if not path.exists(): + print(f"error: {path} not found", file=sys.stderr) + sys.exit(1) + return set(pattern.findall(path.read_text())) + + +def report(kind, missing_in_helm, missing_in_conf, consequence): + problems = 0 + if missing_in_helm: + problems += len(missing_in_helm) + print(f"\n {kind} declared in nginx.conf but MISSING from the Helm ConfigMap:") + for name in sorted(missing_in_helm): + print(f" - {name}") + print(f" {consequence}") + print(f" Fix: add it to {HELM_CONFIGMAP.relative_to(ROOT)}") + if missing_in_conf: + problems += len(missing_in_conf) + print(f"\n {kind} in the Helm ConfigMap but MISSING from nginx.conf:") + for name in sorted(missing_in_conf): + print(f" - {name}") + print(" The compose stack and the image will not have it.") + print(f" Fix: add it to {NGINX_CONF.relative_to(ROOT)}") + return problems + + +def main(): + conf_env = extract(NGINX_CONF, ENV_RE) + helm_env = extract(HELM_CONFIGMAP, ENV_RE) + conf_dicts = extract(NGINX_CONF, DICT_RE) + helm_dicts = extract(HELM_CONFIGMAP, DICT_RE) + + print("Helm / nginx.conf drift check") + print("=" * 30) + print(f" env declarations : {len(conf_env)} in nginx.conf, {len(helm_env)} in the chart") + print(f" shared dicts : {len(conf_dicts)} in nginx.conf, {len(helm_dicts)} in the chart") + + problems = 0 + problems += report( + "env declarations", + conf_env - helm_env, + helm_env - conf_env, + "Consequence: os.getenv() returns nil in Lua on Kubernetes, so the setting is ignored.", + ) + problems += report( + "lua_shared_dict", + conf_dicts - helm_dicts, + helm_dicts - conf_dicts, + "Consequence: ngx.shared. is nil on Kubernetes, so the feature is silently inert.", + ) + + print() + if problems: + print(f"FAILED: {problems} declaration(s) out of sync.") + return 1 + print("The chart's nginx.conf matches the image's.") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) From 4e2aeb261f196ba16aca20188a2a259e5fca6615 Mon Sep 17 00:00:00 2001 From: Martin Dobrev Date: Fri, 7 Aug 2026 15:55:56 +0100 Subject: [PATCH 3/4] fix: Address Copilot review on PR #40 Seven findings, all valid. Two of them are holes in work from this same branch that I had reported as verified. The drift checker had a blind spot. Its env regex was [A-Z_]-only, so it printed "env declarations: identical" while nginx.conf's three lowercase proxy declarations -- http_proxy, https_proxy, no_proxy -- were missing from the Helm ConfigMap. A gate that reports success over real drift is worse than no gate. Regex widened to accept either case, the three declarations added, and the count went 26 -> 29. detectionFlag() mangled unprefixed flags. It stripped everything up to the first colon, turning kw:viagra into viagra, which matches no suppression. Flags are usually profile-prefixed, but defense-line flags are merged into the result without a prefix, so both that rule and the suggested "strip only if the remainder still has a colon" are wrong -- the latter on single-segment flags like legacy:thread_error. The decision is now made against the profile names the API already reports in top_rules: a leading segment is stripped only when it is one of them. The suppression audit log was invisible. It was at INFO while the default error_log level is warn, so "logged, not silent" was not true. Raising every hit to WARN would be its own problem -- a suppression exists because its rule fires often. So apply() now reports whether it actually changed the verdict, and only that case logs at WARN: "SUPPRESSED (block prevented)". Dropping a flag from a node that was not going to block stays at INFO. Verified the WARN line appears at the default level. Idempotency now comes from HSET, not a best-effort scan. HSET returns 1 for a created field and 0 for a replaced one; the earlier read_all() scan could misreport under a concurrent write. The scan remains only for the 200-entry cap. Verified 201 then 200, with one entry stored. get_all() decoded the same JSON on every defense node. Now cached per worker and keyed on the raw blob, so a redis_sync write is picked up on the next call rather than after a TTL -- a stale suppression is either traffic wrongly blocked or wrongly allowed, and neither should wait. A spec drives three successive config changes to prove the cache turns over. OpenAPI was under-specified: POST /suppressions had no requestBody (generated types marked it never) and omitted the 200 idempotent response, DELETE /suppressions documented no body while returning JSON, and DELETE /suppressions/{id} was missing entirely despite existing in the router and in RBAC. Gates: 75 unit specs, helm drift clean at 29/29 and 14/14, config contracts hold, contract check 12 endpoints, integration 42 passed / 2 known gaps / 0 failed, typecheck 0 errors, lint 0 errors / 59 warnings. --- admin-ui/src/api/generated.ts | 88 +++++++++++++++++-- admin-ui/src/pages/shadow/ShadowMode.tsx | 36 ++++++-- docs/openapi.yaml | 74 ++++++++++++++-- .../templates/openresty-configmap.yaml | 3 + openresty/lua/api_handlers/suppressions.lua | 16 ++-- openresty/lua/defense_profile_executor.lua | 20 +++-- openresty/lua/suppressions.lua | 38 +++++--- openresty/spec/suppressions_spec.lua | 40 +++++++++ scripts/check-helm-drift.py | 5 +- 9 files changed, 276 insertions(+), 44 deletions(-) diff --git a/admin-ui/src/api/generated.ts b/admin-ui/src/api/generated.ts index 8b10ae2..2782024 100644 --- a/admin-ui/src/api/generated.ts +++ b/admin-ui/src/api/generated.ts @@ -237,18 +237,39 @@ export interface paths { path?: never; cookie?: never; }; - requestBody?: never; + requestBody: { + content: { + "application/json": { + /** @description Exact detection flag, or a trailing "*" for a family (kw:*). This is the flag the mechanism emits, without the profile prefix that appears on recorded shadow decisions. */ + flag: string; + /** + * @default global + * @enum {string} + */ + scope_type?: "global" | "vhost" | "endpoint"; + /** @description Required unless scope_type is global. */ + scope_id?: string; + reason?: string; + }; + }; + }; responses: { + /** @description Already present. The id is a digest of scope and flag, so re-adding the same suppression updates it rather than creating a duplicate. */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["SuppressionCreated"]; + }; + }; /** @description Created */ 201: { headers: { [name: string]: unknown; }; content: { - "application/json": { - suppression?: components["schemas"]["Suppression"]; - created?: boolean; - }; + "application/json": components["schemas"]["SuppressionCreated"]; }; }; /** @description Invalid input. Includes a global "*", which would disable all detection and is refused deliberately. */ @@ -272,6 +293,58 @@ export interface paths { responses: { /** @description Cleared */ 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": { + cleared: boolean; + }; + }; + }; + }; + }; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/suppressions/{id}": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post?: never; + /** Remove one suppression */ + delete: { + parameters: { + query?: never; + header?: never; + path: { + id: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Removed */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": { + deleted: boolean; + id: string; + }; + }; + }; + /** @description No suppression with that id */ + 404: { headers: { [name: string]: unknown; }; @@ -591,6 +664,11 @@ export interface components { created_at?: number; created_by?: string; }; + SuppressionCreated: { + suppression: components["schemas"]["Suppression"]; + /** @description False when the suppression already existed and was updated. */ + created: boolean; + }; ShadowDecision: { /** @description Unix time the decision was made */ ts: number; diff --git a/admin-ui/src/pages/shadow/ShadowMode.tsx b/admin-ui/src/pages/shadow/ShadowMode.tsx index b632219..8c81629 100644 --- a/admin-ui/src/pages/shadow/ShadowMode.tsx +++ b/admin-ui/src/pages/shadow/ShadowMode.tsx @@ -47,14 +47,23 @@ function formatWhen(ts: number): string { } /** - * Recorded flags are profile-prefixed (`legacy:kw:viagra`) because a decision can - * come from any profile, but a suppression matches the flag the mechanism itself - * emits (`kw:viagra`). Strip the profile segment so the suppression form is - * prefilled with something that will actually match. + * Recorded flags are usually profile-prefixed (`legacy:kw:viagra`), but not + * always: defense-line flags are merged into the result without a prefix. A + * suppression matches the flag the mechanism itself emits, so the prefix has to + * come off -- and only when it really is a prefix. + * + * Stripping up to the first colon unconditionally mangles an unprefixed + * `kw:viagra` into `viagra`, which matches nothing. Requiring the remainder to + * still contain a colon fails the other way, on single-segment flags like + * `legacy:thread_error`. So the decision is made against the profile names the + * API already reports in top_rules: strip a leading segment only if it is one + * of them. */ -function detectionFlag(recorded: string): string { +function detectionFlag(recorded: string, profileNames: Set): string { const firstColon = recorded.indexOf(':') - return firstColon === -1 ? recorded : recorded.slice(firstColon + 1) + if (firstColon === -1) return recorded + const head = recorded.slice(0, firstColon) + return profileNames.has(head) ? recorded.slice(firstColon + 1) : recorded } /** @@ -65,10 +74,12 @@ function CountBars({ items, empty, suppressScope, + profileNames, }: { items: ShadowCount[] empty: string suppressScope?: string + profileNames?: Set }) { if (!items?.length) { return

{empty}

@@ -82,16 +93,16 @@ function CountBars({ {item.name} {item.count} - {suppressScope && ( + {suppressScope && profileNames && (