feat: Add backup/restore, signature testing, and defense profile fixes by mclueppers · Pull Request #31 · dobrevit/forms-waf

mclueppers · 2026-01-07T14:39:49Z

Backup & Restore System:

Add backup API for export/import of WAF configuration
New BackupRestore settings page with export/import dialogs
Support selective entity export and merge/replace import modes

Attack Signature Testing:

Add test endpoint for validating signatures against sample data
New SignatureTestDialog component for interactive testing

Form Processing & Charset:

Add charset.lua module for non-UTF-8 form data conversion
Fix multipart file detection via Content-Type header (not just filename)
Sanitize user-supplied data in HTTP headers for UTF-8 compliance

Defense Profile Monitoring Mode:

Fix profiles returning "allow" instead of "block" in monitoring mode
Preserve would-block state to ensure correct "monitored" metrics
Set HAProxy request headers in monitoring mode for rate limiting

WAF Logging:

Use nginx variables ($waf_spam_score, etc.) instead of response headers
Only expose debug headers to clients when explicitly enabled

Dashboard & Stats:

Fix endpoint count to include vhost-specific endpoints
Align React Query cache keys for consistent data display

Infrastructure:

Add musl-dev and build-base for lua-iconv compilation in Docker
Add vhost-specific keyword checking with deduplication

Copilot

Pull request overview

This PR adds comprehensive backup/restore functionality, attack signature testing capabilities, and critical fixes for defense profile monitoring mode. It also introduces charset conversion for non-UTF-8 form data and improves logging by using nginx variables instead of response headers.

Key Changes:

Backup/restore system with selective export/import and multiple merge modes
Attack signature testing endpoint with interactive UI dialog
Fixed defense profiles incorrectly returning "allow" in monitoring mode (preserves would-block state)
UTF-8 charset conversion for form data with lua-iconv integration
Logging improvements using nginx variables for internal tracking

Reviewed changes

Copilot reviewed 25 out of 26 changed files in this pull request and generated 16 comments.

Show a summary per file

File	Description
openresty/lua/waf_handler.lua	Integrates charset sanitization, adds vhost keyword checking, sets nginx logging variables, and fixes monitoring mode header propagation
openresty/lua/charset.lua	New module providing charset detection, UTF-8 conversion, and header sanitization
openresty/lua/form_parser.lua	Adds charset conversion support and improves multipart file detection via Content-Type headers
openresty/lua/defense_profile_executor.lua	Fixes monitoring mode to preserve would-block state instead of overwriting with allow
openresty/lua/defense_mechanisms.lua	Integrates charset sanitization for filtered field names
openresty/lua/api_handlers/backup.lua	New comprehensive backup/restore API with export, import, and validation endpoints
openresty/lua/api_handlers/attack_signatures.lua	Adds signature testing endpoint for validating patterns against sample data
openresty/lua/redis_sync.lua	Fixes endpoint count to include both global and vhost-specific endpoints
openresty/lua/rbac.lua	Adds backup resource permissions for export/import operations
openresty/lua/admin_api.lua	Registers backup handler endpoints
openresty/conf/nginx.conf	Changes logging to use nginx variables instead of response headers
openresty/Dockerfile	Installs lua-iconv and build dependencies for charset conversion
admin-ui/src/pages/settings/BackupRestore.tsx	New backup/restore settings page with export and import dialogs
admin-ui/src/components/backup/*.tsx	Export and import dialog components with validation and progress tracking
admin-ui/src/components/attack-signatures/SignatureTestDialog.tsx	Interactive signature testing UI with sample data inputs
admin-ui/src/pages/security/AttackSignatureEditor.tsx	Adds test button to signature editor
admin-ui/src/pages/vhosts/VhostForm.tsx	Normalizes defense_profiles array to handle Lua cjson encoding quirks
admin-ui/src/pages/Dashboard.tsx	Updates query key for endpoint list to ensure cache consistency
admin-ui/src/api/client.ts	Adds backup API client methods and signature testing endpoint
admin-ui/src/api/types.ts	Defines TypeScript types for backup structures and test results
admin-ui/src/components/ui/checkbox.tsx	New checkbox UI component for backup entity selection

Files not reviewed (1)

admin-ui/package-lock.json: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-07T14:49:08Z

openresty/lua/api_handlers/attack_signatures.lua

+                local ok, matched = pcall(function()
+                    return ua:match(pattern)
+                end)
+                if ok and matched then
+                    table.insert(results.matches, {
+                        type = "fingerprint",
+                        pattern_type = "blocked_user_agents",
+                        pattern = pattern,
+                        matched_value = ua,
+                        action = "block"
+                    })
+                    results.would_block = true
+                end
+            end


The test endpoint uses pcall to safely execute pattern matching (e.g., line 434), which is good for preventing crashes from invalid regex patterns. However, when the pcall fails (ok is false), the error is silently ignored and the pattern is treated as non-matching. This could hide legitimate issues with signature patterns. Consider logging pattern matching errors so administrators can identify and fix broken patterns.

Copilot · 2026-01-07T14:49:09Z

openresty/lua/waf_handler.lua

+                    local kw_lower = kw:lower()
+                    if not checked_blocked[kw_lower] and combined_text:find(kw_lower, 1, true) then


In the keyword checking logic, line 561 uses string.find with plain text search (true as 4th argument), but if kw_lower contains pattern characters like %, -, etc., this won't cause issues since plain=true. However, the case conversion happens after the pattern check which could lead to inconsistent behavior if keywords have different cases. The logic is correct, but consider documenting why case conversion happens before the find operation to prevent future bugs.

Copilot · 2026-01-07T14:49:09Z

openresty/lua/defense_profile_executor.lua

+    -- In monitoring mode, preserve would-block state (don't overwrite block with allow)
+    if not (context.is_monitoring_mode and context.final_action == "block") then
+        context.final_action = "allow"
+    end


The monitoring mode preservation logic checks if final_action is "block" before potentially overwriting it with "allow". However, this creates a scenario where an "allow" action node in monitoring mode cannot override a previous "block" decision. While this might be intentional to preserve would-block state, it means that defense profiles with explicit "allow" actions later in the flow won't work as expected in monitoring mode. Consider documenting this behavior or adding a flag to distinguish between "would block" and actual block decisions.

Copilot · 2026-01-07T14:49:09Z

admin-ui/src/components/backup/ImportDialog.tsx

+    if (file && file.type === 'application/json') {
+      const input = fileInputRef.current
+      if (input) {
+        const dataTransfer = new DataTransfer()
+        dataTransfer.items.add(file)
+        input.files = dataTransfer.files
+        const changeEvent = new Event('change', { bubbles: true })
+        input.dispatchEvent(changeEvent)


The import dialog file drop handler on line 112 checks if the file type is 'application/json', but not all browsers set this MIME type correctly for .json files. Some browsers might use 'text/json' or not set a type at all for dropped files. Consider checking the file extension as a fallback or accepting any file and validating the content after parsing.

Suggested change

if (file && file.type === 'application/json') {

const input = fileInputRef.current

if (input) {

const dataTransfer = new DataTransfer()

dataTransfer.items.add(file)

input.files = dataTransfer.files

const changeEvent = new Event('change', { bubbles: true })

input.dispatchEvent(changeEvent)

if (file) {

const mimeType = file.type

const fileName = (file.name || '').toLowerCase()

const isJsonFile =

mimeType === 'application/json' ||

mimeType === 'text/json' ||

fileName.endsWith('.json')

if (isJsonFile) {

const input = fileInputRef.current

if (input) {

const dataTransfer = new DataTransfer()

dataTransfer.items.add(file)

input.files = dataTransfer.files

const changeEvent = new Event('change', { bubbles: true })

input.dispatchEvent(changeEvent)

}

Copilot · 2026-01-07T14:49:10Z

openresty/lua/charset.lua

+    -- Replace control characters (0x00-0x1F except tab/newline) and high bytes (0x7F-0xFF)
+    return str:gsub("[%z\1-\8\11\12\14-\31\127-\255]", "_")


The regex pattern includes control characters that should be replaced, but it uses an incorrect range. The pattern [%z\1-\8\11\12\14-\31\127-\255] includes \11 (tab) and \12 (form feed) which are typically allowed in certain contexts. The comment on line 103 says "except tab/newline" but tab (0x09) is \9 not \11, and form feed (0x0C) is \12. This mismatch between the comment and implementation could lead to incorrect sanitization.

Copilot · 2026-01-07T14:49:12Z

openresty/lua/form_parser.lua

+                -- If a Content-Type header is present for this part, treat as file
+                -- (regular form fields don't have Content-Type headers)
+                local content_type = res[2] or ""
+                -- Only treat as file if it's not a text content type
+                if not content_type:match("^text/") then
+                    current_is_file = true


The multipart file detection logic checks for Content-Type header and excludes text/* types. However, this could incorrectly treat legitimate text/plain fields as files if they have a Content-Type header. Regular form fields typically don't have Content-Type headers, but if a client sends one for a text field, it would be treated as a file. Consider refining this logic to only treat as file if Content-Type is present AND it's a binary type, or if filename is present.

Suggested change

-- If a Content-Type header is present for this part, treat as file

-- (regular form fields don't have Content-Type headers)

local content_type = res[2] or ""

-- Only treat as file if it's not a text content type

if not content_type:match("^text/") then

current_is_file = true

-- Refine file detection:

-- - Prefer filename (handled above).

-- - Otherwise, only treat as file if Content-Type clearly indicates

-- a binary payload (e.g. image/*, audio/*, video/*, octet-stream).

local content_type = (res[2] or ""):lower()

-- Do not override filename-based decision

if not current_filename and content_type ~= "" then

-- Ignore obvious textual types

local is_textual =

content_type:match("^text/") ~= nil or

content_type:match("^application/json") ~= nil or

content_type:match("^application/x%-www%-form%-urlencoded") ~= nil

-- Heuristic for clearly binary types

local is_binary =

content_type:match("^image/") ~= nil or

content_type:match("^audio/") ~= nil or

content_type:match("^video/") ~= nil or

content_type:match("^application/octet%-stream") ~= nil

if is_binary and not is_textual then

current_is_file = true

end

Copilot · 2026-01-07T14:49:12Z

admin-ui/src/pages/Dashboard.tsx

-    queryFn: endpointsApi.list,
+    queryKey: ['endpoints', 'all'],
+    queryFn: () => endpointsApi.list(),
+    staleTime: 0, // Always refetch on mount to ensure accurate counts


The Dashboard component sets staleTime to 0 on line 64, forcing the endpoints query to always refetch on mount. While this ensures accurate counts, it could lead to unnecessary API calls if the dashboard is frequently navigated to. Consider using a short staleTime (e.g., 30 seconds) instead of 0 to balance freshness with performance, or use query invalidation after relevant mutations.

Suggested change

staleTime: 0, // Always refetch on mount to ensure accurate counts

staleTime: 30000, // Cache for 30s to balance fresh counts with fewer API calls

Copilot · 2026-01-07T14:49:13Z

openresty/lua/api_handlers/backup.lua

+    -- Helper to import entities with config prefix
+    local function import_entities(entity_type, entities, entity_key)
+        if not entities or type(entities) ~= "table" or not entities[1] then
+            return
+        end
+
+        results.imported[entity_type] = 0
+        results.skipped[entity_type] = 0
+        results.updated[entity_type] = 0
+        results.errors[entity_type] = {}
+
+        for _, entity in ipairs(entities) do
+            if not entity.id then
+                table.insert(results.errors[entity_type], "Entity missing 'id' field")
+                goto next_entity
+            end
+
+            local key = entity_key.config_prefix .. entity.id
+            local exists = red:exists(key)
+
+            if mode == "merge" then
+                -- Skip existing
+                if exists == 1 then
+                    results.skipped[entity_type] = results.skipped[entity_type] + 1
+                    goto next_entity
+                end
+            elseif mode == "replace" then
+                -- Always overwrite
+            elseif mode == "update" then
+                -- Only update if exists, create if not
+            end
+
+            -- Store entity
+            local entity_json = cjson.encode(entity)
+            if not entity_json then
+                table.insert(results.errors[entity_type], "Failed to encode entity: " .. entity.id)
+                goto next_entity
+            end
+
+            local ok, err = red:set(key, entity_json)
+            if not ok then
+                table.insert(results.errors[entity_type], string.format("Failed to save %s: %s", entity.id, err or "unknown"))
+                goto next_entity
+            end
+
+            -- Add to index if exists
+            if entity_key.index then
+                local priority = entity.priority or 0
+                red:zadd(entity_key.index, priority, entity.id)
+            end
+
+            if exists == 1 then
+                results.updated[entity_type] = results.updated[entity_type] + 1
+            else
+                results.imported[entity_type] = results.imported[entity_type] + 1
+            end
+
+            ::next_entity::
+        end
+    end


The import function doesn't check for Redis errors after individual SET and ZADD operations during entity import. If Redis becomes unavailable mid-import or a write fails, the import could be left in a partial state without proper error reporting. While errors are caught and added to the errors array, the function continues processing other entities. Consider wrapping the import in a transaction or implementing rollback on failure to maintain data consistency.

Copilot · 2026-01-07T14:49:13Z

openresty/lua/form_parser.lua

        return nil, "unsupported content type"
    end

+    -- Convert form data to UTF-8 if a non-UTF-8 charset was specified


The charset conversion logic on lines 264-266 excludes ASCII from conversion, but ASCII is a subset of UTF-8. The check source_charset ~= "ASCII" means ASCII data won't be converted, which is correct since ASCII is already compatible with UTF-8. However, the comment on line 263 says "Convert form data to UTF-8 if a non-UTF-8 charset was specified" which could be misleading. Consider clarifying the comment to mention that ASCII is also skipped as it's UTF-8 compatible.

Suggested change

-- Convert form data to UTF-8 if a non-UTF-8 charset was specified

-- Convert form data to UTF-8 when a non-UTF-8, non-ASCII charset was specified

-- (ASCII is skipped because it is a subset of UTF-8 and already compatible)

Copilot · 2026-01-07T14:49:13Z

openresty/Dockerfile

    opm get anjia0532/lua-resty-maxminddb && \
    opm get zmartzone/lua-resty-openidc && \
    luarocks-5.1 install lua-resty-woothee && \
+    luarocks-5.1 install lua-iconv ICONV_DIR=/usr LUA_INCDIR=/usr/local/openresty/luajit/include/luajit-2.1 && \


The luarocks-5.1 install lua-iconv step introduces a new unpinned third-party build-time dependency fetched from the public LuaRocks registry, which can be silently updated upstream and pulled into production images on rebuild. If the lua-iconv package or registry is compromised, an attacker could inject arbitrary Lua/C code into the WAF image with full access to request data and any in-container secrets. To reduce supply chain risk, pin this dependency to an immutable version or verified artifact (e.g., specific version + checksum or a vetted internal mirror) rather than the floating latest release.

Backup & Restore System: - Add backup API for export/import of WAF configuration - New BackupRestore settings page with export/import dialogs - Support selective entity export and merge/replace import modes Attack Signature Testing: - Add test endpoint for validating signatures against sample data - New SignatureTestDialog component for interactive testing Form Processing & Charset: - Add charset.lua module for non-UTF-8 form data conversion - Fix multipart file detection via Content-Type header (not just filename) - Sanitize user-supplied data in HTTP headers for UTF-8 compliance Defense Profile Monitoring Mode: - Fix profiles returning "allow" instead of "block" in monitoring mode - Preserve would-block state to ensure correct "monitored" metrics - Set HAProxy request headers in monitoring mode for rate limiting WAF Logging: - Use nginx variables ($waf_spam_score, etc.) instead of response headers - Only expose debug headers to clients when explicitly enabled Dashboard & Stats: - Fix endpoint count to include vhost-specific endpoints - Align React Query cache keys for consistent data display Infrastructure: - Add musl-dev and build-base for lua-iconv compilation in Docker - Add vhost-specific keyword checking with deduplication

mclueppers requested a review from Copilot January 7, 2026 14:40

Copilot started reviewing on behalf of mclueppers January 7, 2026 14:40 View session

Copilot AI reviewed Jan 7, 2026

View reviewed changes

mclueppers force-pushed the fix/defence-profile-polish branch from 63bb0bb to 5a9bf72 Compare January 7, 2026 15:06

mclueppers merged commit d3b3359 into main Jan 7, 2026
3 checks passed

mclueppers deleted the fix/defence-profile-polish branch January 7, 2026 15:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add backup/restore, signature testing, and defense profile fixes#31

feat: Add backup/restore, signature testing, and defense profile fixes#31
mclueppers merged 1 commit intomainfrom
fix/defence-profile-polish

mclueppers commented Jan 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 7, 2026

Uh oh!

Copilot AI Jan 7, 2026

Uh oh!

Copilot AI Jan 7, 2026

Uh oh!

Copilot AI Jan 7, 2026

Uh oh!

Copilot AI Jan 7, 2026

Uh oh!

Copilot AI Jan 7, 2026

Uh oh!

Copilot AI Jan 7, 2026

Uh oh!

Copilot AI Jan 7, 2026

Uh oh!

Copilot AI Jan 7, 2026

Uh oh!

Copilot AI Jan 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		local kw_lower = kw:lower()
		if not checked_blocked[kw_lower] and combined_text:find(kw_lower, 1, true) then

		-- Replace control characters (0x00-0x1F except tab/newline) and high bytes (0x7F-0xFF)
		return str:gsub("[%z\1-\8\11\12\14-\31\127-\255]", "_")

-                -- If a Content-Type header is present for this part, treat as file
-                -- (regular form fields don't have Content-Type headers)
-                local content_type = res[2] or ""
-                -- Only treat as file if it's not a text content type
-                if not content_type:match("^text/") then
-                    current_is_file = true
+                -- Refine file detection:
+                -- - Prefer filename (handled above).
+                -- - Otherwise, only treat as file if Content-Type clearly indicates
+                --   a binary payload (e.g. image/*, audio/*, video/*, octet-stream).
+                local content_type = (res[2] or ""):lower()
+                -- Do not override filename-based decision
+                if not current_filename and content_type ~= "" then
+                    -- Ignore obvious textual types
+                    local is_textual =
+                        content_type:match("^text/") ~= nil or
+                        content_type:match("^application/json") ~= nil or
+                        content_type:match("^application/x%-www%-form%-urlencoded") ~= nil
+                    -- Heuristic for clearly binary types
+                    local is_binary =
+                        content_type:match("^image/") ~= nil or
+                        content_type:match("^audio/") ~= nil or
+                        content_type:match("^video/") ~= nil or
+                        content_type:match("^application/octet%-stream") ~= nil
+                    if is_binary and not is_textual then
+                        current_is_file = true
+                    end

	staleTime: 0, // Always refetch on mount to ensure accurate counts
	staleTime: 30000, // Cache for 30s to balance fresh counts with fewer API calls

	-- Convert form data to UTF-8 if a non-UTF-8 charset was specified
	-- Convert form data to UTF-8 when a non-UTF-8, non-ASCII charset was specified
	-- (ASCII is skipped because it is a subset of UTF-8 and already compatible)

Conversation

mclueppers commented Jan 7, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant