Conversation
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Matrix Notification Plugin (New)Summary: New plugin added to send attack denial notifications to Matrix chat rooms via the Matrix Client-Server API. Implementation includes async delivery and IP/ASN enrichment. Critical runtime bug and missing version tracking present. Files Added
Plugin Metadata
Critical Issues
Functional Changes
Security Observations
ConfigurationBoolean settings correctly use strict regex Documentation & Deployment
WalkthroughA new Matrix notification plugin is introduced for BunkerWeb, enabling the system to send attack notifications to a configured Matrix room. The implementation includes core Lua logic, plugin configuration, documentation, and UI integration for health monitoring. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@matrix/matrix.lua`:
- Around line 106-136: The send path currently continues after http_new() or
httpc:request_uri() failures, which causes httpc:close() and res.status
dereferences on nil; update the logic in the Matrix send function(s) to
immediately return an error when http_new() fails (http_new -> httpc nil) and
likewise after request_uri() fails (res nil), ensuring you do not call
httpc:close() or access res.status in those failure branches; specifically, in
the code using http_new(), httpc, httpc:request_uri, httpc:close and res.status
(also apply the same fix to the second send path around the block referenced at
185-203), log the error and return early before touching httpc or res.
- Around line 75-85: When MATRIX_INCLUDE_HEADERS is enabled in matrix.lua, avoid
forwarding raw headers in the loop that appends to data["formatted_body"] and
data["body"]; instead implement an allow-list (e.g., only include common safe
headers like "User-Agent", "Accept", "Host") or explicitly redact sensitive
names (e.g., "Authorization", "Cookie", "Set-Cookie", "X-CSRF-Token",
"Proxy-Authorization", "Authentication") before serialising, and in the for
header,value in pairs(headers) block replace sensitive values with a placeholder
like "[REDACTED]" (or skip adding those headers entirely) so no bearer tokens,
cookies, CSRF tokens, or secrets are sent to Matrix.
- Around line 48-50: The country lookup fallback has a typo and uses an
undefined variable: replace "elf.logger" with "self.logger", reference the IP
with "self.ctx.bw.remote_addr" in the log message (instead of "remote_addr"),
and ensure you set a safe fallback value (e.g. assign country = "Country
unknown") after logging so callers of get_country handling the nil case don't
raise; look for get_country, self.ctx.bw.remote_addr, self.logger and the local
variable country in the surrounding code to apply these changes.
- Around line 124-131: The Matrix HTTP calls use httpc:request_uri() without
timeouts; before each request_uri call (e.g., the PUT call that sends post_data
to url using access_token and the other request_uri call later) call
httpc:set_timeout(1000) or httpc:set_timeouts(...) to enforce a 1s (or
configured) timeout so the worker thread won't block; update both places that
call httpc:request_uri() to set the timeout on the httpc instance immediately
before invoking request_uri().
- Around line 111-115: The txn_id currently set via tostring(os.time()) is not
collision-resistant; replace it with a per-send unique id and update both places
where txn_id is generated/used (the variable named txn_id in matrix.lua). For
example, build txn_id by concatenating os.time() with a higher-resolution or
unique per-request value: use ngx.var.msec if available (tostring(os.time()) ..
"-" .. ngx.var.msec), or ngx.var.request_id when present, or combine
ngx.now()*1000 with a short random suffix to guarantee uniqueness across rapid
successive sends; ensure both locations that set txn_id use the same new
generation logic so Matrix idempotency keys are unique per send.
- Around line 70-85: Sanitise and flatten request-derived inputs before
concatenating into data["formatted_body"] and data["body"]: run request_method,
remote_addr, request_host, self.ctx.bw.uri, reason, reason_data and every header
name/value through an HTML-escape helper (e.g., escape_html) for formatted_body
and tostring for plain body, and when reading headers from ngx_req.get_headers()
handle table values by flattening with table.concat(v, ", ") (or tostring on
non-tables) so duplicate headers do not cause concatenation errors; apply this
transformation where data["formatted_body"] and data["body"] are built and when
iterating headers returned by ngx_req.get_headers().
In `@matrix/README.md`:
- Around line 5-15: The Table of contents contains a stale entry linking to
"#todo" that does not exist; open the README.md section containing the bulleted
list (the lines under "Table of contents") and remove the "- [TODO](`#todo`)" list
item so the TOC matches the actual headings and keeps the documentation concise
and accurate.
- Around line 34-50: Update the Docker examples in matrix/README.md to reference
the 1.6 image series: replace all occurrences of the image tag
"bunkerity/bunkerweb:1.5.10" (seen in the bunkerweb service blocks) with the
appropriate 1.6 tag (e.g., "bunkerity/bunkerweb:1.6" or the exact 1.6.x release
you intend to document) so examples match this PR's 1.6 compatibility; ensure
both instances mentioned in the diff are updated.
- Around line 86-93: The settings table in README.md has drifted from
matrix/plugin.json; rerun the JSON-to-Markdown generator
(.tests/misc/json2md.py) against matrix/plugin.json to regenerate the table (so
defaults like MATRIX_ACCESS_TOKEN show as an empty string), replace the manually
edited table in README.md with the generated output, and commit the regenerated
README instead of hand-editing the table.
In `@matrix/ui/actions.py`:
- Around line 18-21: The code assigns str(e) to ret["error"], exposing
potentially sensitive details; change this so the full exception remains only in
server logs (keep logger.debug(format_exc()) and logger.error(f"Failed to get
matrix ping: {e}") for diagnostics) but replace ret["error"] with a generic,
non-sensitive marker such as "Unable to retrieve matrix ping" or
"internal_error" (update the assignment to ret["error"] accordingly) — locate
the try/except handling around the ping path where ret["error"] is set and
remove any raw exception string from the payload.
- Around line 29-30: The function matrix(**kwargs) currently returns None which
violates the plugin main-page contract; update the matrix function to return an
empty dict ({}). Locate the matrix function definition and replace the
pass/implicit None with a return {} so it matches other plugin entry points
(e.g., clamav, coraza, crowdsec, discord) and ensures callers receive a
dictionary.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 753a0cbf-db03-4998-b08e-b65f9b264c6e
📒 Files selected for processing (6)
README.mdmatrix/README.mdmatrix/matrix.luamatrix/plugin.jsonmatrix/ui/actions.pymatrix/utils.lua
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Documentation should be concise, accurate, and written in British English:
- Keep the structure clear with a sensible heading hierarchy.
- Each plugin README contains a settings table generated from
plugin.jsonvia.tests/misc/json2md.py; regenerate it whenever settings change rather than hand-editing the table.- The repository README shows a compatibility badge tied to the plugins-collection version in
COMPATIBILITY.json— keep it in sync with any version bump.- Prefer concrete instructions, accurate examples, and explicit prerequisites (Docker,
sudo, required env vars likeVIRUSTOTAL_API_KEY).- When a PR changes behaviour, defaults, packaging, or security posture, ask for the corresponding documentation update.
Files:
README.mdmatrix/README.md
**/*.lua
⚙️ CodeRabbit configuration file
**/*.lua: Lua code runs on OpenResty inside the BunkerWeb nginx container and sits on the request hot path:
- Every plugin subclasses
bunkerweb.pluginviamiddleclass:local <name> = class("<name>", plugin)andplugin.initialize(self, "<id>", ctx)ininitialize. Hook methods (init_worker,access,log,preread, ...) must returnself:ret(ok_bool, msg, [http_status]). To deny a request, returnself:ret(true, "reason", utils.get_deny_status()).- Gate expensive work at
init_workerwithutils.has_variable("USE_<PLUGIN>", "yes")and skip whenself.is_loadingis true, matching the pattern used acrossclamav.lua,coraza.lua, andvirustotal.lua.- Use local variables and local module tables; avoid globals. Cache
ngx.var.*andngx.req.*values in locals instead of re-reading them repeatedly.- Precompile regular expressions in module-level locals; never compile inside request loops. For
ngx.re.match/find/gmatch/sub, pass the option string"jo"(jenables PCRE JIT,ocompiles the pattern once and caches it), anchor patterns with^...$when a full match is intended, and cap input length before matching to prevent ReDoS.- Validate and sanitise all request-derived input. Never evaluate request-derived code via
load,loadstring, or similar mechanisms.- Use
ngx.socketfor raw TCP (see the ClamAV INSTREAM pattern inclamav.lua) andresty.httpfor HTTP upstreams (seevirustotal.luaandcoraza.lua). Preferresty.uploadfor streaming request bodies —clamav.luais the reference.- Cache upstream scan results by a strong digest of the body (SHA-512 in
virustotal.lua) rather than by filename or weaker hashes; mind cache key length.- Shared-dictionary read-modify-write sequences are race-prone; prefer atomic operations such as
incror explicit locking where correctness matters.- Never log request bodies, cookies, bearer tokens, webhook secrets, or API keys.
- Use
pcallor explicit error...
Files:
matrix/utils.luamatrix/matrix.lua
**/plugin.json
⚙️ CodeRabbit configuration file
**/plugin.json:plugin.jsonfiles define the settings schema that BunkerWeb reads to register settings and render the UI:
- The top-level fields
id,name,version, andstream(yes/no/partial) must stay consistent with the plugin directory name and its Lua module.- Ensure setting IDs remain stable unless there is an intentional breaking change documented in the PR.
- Each setting must declare
context(globalormultisite),default,help,id,label,regex, andtype. Regex validators must be anchored where appropriate, compile cleanly, and avoid catastrophic backtracking. Default values must satisfy their own validators.USE_<PLUGIN>toggles are the standard gate forinit_workershort-circuiting; keep them in sync with the Lua implementation.- Bump the per-plugin
versionfield through./misc/update_version.sh <new_version>so everyplugin.jsonand the README badge move together. Do not edit the version in a singleplugin.jsonby hand.- If a PR changes a setting ID, type, context, accepted value shape, or compatibility behaviour, require migration notes and confirm that the README settings table is regenerated via
.tests/misc/json2md.py.
Files:
matrix/plugin.json
**/*.py
⚙️ CodeRabbit configuration file
**/*.py: Follow BunkerWeb's Python standards and security posture:
- Use snake_case for functions and variables, PascalCase for classes, and provide concise, accurate docstrings for public classes, functions, and methods.
- Respect Black formatting with a 160-character line limit (
pyproject.toml) and flake8 with--max-line-length=250 --ignore=E266,E402,E722,W503from.pre-commit-config.yaml.- Catch specific exceptions; never use bare
except:. CatchingBaseException/Exceptionis acceptable only at explicit process boundaries (for example UIpre_renderhooks that must degrade gracefully) when the code logs enough context and either re-raises, returns an error status, or terminates safely.ui/actions.pyusesBaseExceptiondeliberately — do not regress that to a bareexcept:.- Never invoke the shell from Python via
os.system(...),subprocesswithshell=True,eval, orexec. Pass subprocess arguments as a list and prefer explicit binary paths for privileged operations.- Do not use unsafe binary deserialisation libraries for untrusted data. Use
yaml.safe_load()rather than unsafe YAML loading.- Open files with an explicit encoding (normally
utf-8) and usewithstatements for files, sockets, and temporary resources.- For HTTP clients (
requests,httpx): always set an explicit timeout, validate destination URLs to block RFC1918/loopback/link-local ranges (SSRF), disable automatic redirects to internal hosts, and be careful with proxy settings.- Scrub secrets, tokens, cookies, database URIs, webhook URLs, and
Authorizationheaders from logs. Use the logging framework rather thanprint().- Secrets, API keys, and credentials must never be hard-coded; read them from plugin settings or environment variables.
Files:
matrix/ui/actions.py
*/ui/actions.py
⚙️ CodeRabbit configuration file
*/ui/actions.py: Per-pluginui/actions.pymodules hook into the BunkerWeb web UI:
pre_render(**kwargs)must return a dict of card data and must never raise out to the caller. The existing pattern wraps the body intry/except BaseExceptionand logs viagetLogger("UI")+format_exc(); preserve that posture.- The top-level function named after the plugin (e.g.
discord(**kwargs)) is the main-page entry point; keep it side-effect free unless the plugin genuinely needs server-side state.kwargs["bw_instances_utils"]exposes BW helpers such asget_ping(service); do not assume other keys exist without checking.- Never expose secrets, webhook URLs, API keys, or tokens in the returned card payload — they are rendered into the UI.
- Keep the module importable with only the BunkerWeb UI runtime; do not add heavy third-party dependencies without justification.
Files:
matrix/ui/actions.py
🪛 LanguageTool
matrix/README.md
[grammar] ~23-~23: The verb after “to” should be in the base form as part of the to-infinitive. A verb can take many forms, but the base form is always used in the to-infinitive.
Context: ...s token for the Matrix user you want to sent notifications from. - A room ID where n...
(TO_NON_BASE)
🪛 Luacheck (1.2.0)
matrix/matrix.lua
[warning] 55-55: variable 'err' was previously defined on line 48
(W411)
[warning] 62-62: variable 'err' was previously defined on line 55
(W411)
[warning] 76-76: shadowing definition of variable 'err' on line 62
(W421)
[warning] 97-97: variable 'err' was previously defined on line 62
(W411)
[warning] 194-194: line contains trailing whitespace
(W612)
🪛 markdownlint-cli2 (0.22.0)
matrix/README.md
[warning] 15-15: Link fragments should be valid
(MD051, link-fragments)
| local country, err = get_country(self.ctx.bw.remote_addr) | ||
| if not country then | ||
| elf.logger:log(ERR, "can't get Country of IP " .. remote_addr .. " : " .. err) |
There was a problem hiding this comment.
Fix the logger typo in the country lookup fallback.
Line 50 calls elf.logger, so the first failed country lookup raises instead of falling back to "Country unknown".
Suggested fix
- elf.logger:log(ERR, "can't get Country of IP " .. remote_addr .. " : " .. err)
+ self.logger:log(ERR, "can't get Country of IP " .. remote_addr .. " : " .. err)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| local country, err = get_country(self.ctx.bw.remote_addr) | |
| if not country then | |
| elf.logger:log(ERR, "can't get Country of IP " .. remote_addr .. " : " .. err) | |
| local country, err = get_country(self.ctx.bw.remote_addr) | |
| if not country then | |
| self.logger:log(ERR, "can't get Country of IP " .. remote_addr .. " : " .. err) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@matrix/matrix.lua` around lines 48 - 50, The country lookup fallback has a
typo and uses an undefined variable: replace "elf.logger" with "self.logger",
reference the IP with "self.ctx.bw.remote_addr" in the log message (instead of
"remote_addr"), and ensure you set a safe fallback value (e.g. assign country =
"Country unknown") after logging so callers of get_country handling the nil case
don't raise; look for get_country, self.ctx.bw.remote_addr, self.logger and the
local variable country in the surrounding code to apply these changes.
| data["formatted_body"] = "<p>Denied " .. request_method .. " from <b>" .. remote_addr .. "</b> (" .. country .. " • \"<i>" .. asn_org .. "</i>\" • " .. asn .. ") to " .. request_host .. self.ctx.bw.uri .. "<br>" | ||
| data["formatted_body"] = data["formatted_body"] .. "Reason <b>" .. reason .. "</b> (" .. encode(reason_data or {}) .. ").</p>" | ||
| data["body"] = "Denied " .. request_method .. " from " .. remote_addr .. " (" .. country .. " • \"" .. asn_org .. "\" • " .. asn .. ") to " .. request_host .. self.ctx.bw.uri .. "\n" | ||
| data["body"] = data["body"] .. "Reason " .. reason .. " (" .. encode(reason_data or {}) .. ")." | ||
| -- Add headers if enabled | ||
| if self.variables["MATRIX_INCLUDE_HEADERS"] == "yes" then | ||
| local headers, err = ngx_req.get_headers() | ||
| if not headers then | ||
| data["formatted_body"] = data["formatted_body"] .. "error while getting headers: " .. err | ||
| data["body"] = data["body"] .. "\n error while getting headers: " .. err | ||
| else | ||
| data["formatted_body"] = data["formatted_body"] .. "<table><tr><th>Header</th><th>Value</th></tr>" | ||
| data["body"] = data["body"] .. "\n\n" | ||
| for header, value in pairs(headers) do | ||
| data["formatted_body"] = data["formatted_body"] .. "<tr><td>" .. header .. "</td><td>" .. value .. "</td></tr>" | ||
| data["body"] = data["body"] .. header .. ": " .. value .. "\n" |
There was a problem hiding this comment.
Escape HTML and flatten duplicate headers before concatenation.
Lines 70–80 inject request fields directly into formatted_body without sanitisation. Request-derived fields (request_method, remote_addr, request_host, URI, header names and values) can contain HTML special characters. Repeated HTTP headers from ngx.req.get_headers() arrive as tables, causing concatenation with .. to fail. Escape all dynamic fields for HTML and handle header arrays: local value = type(v) == "table" and table.concat(v, ", ") or tostring(v) before concatenation.
As per coding guidelines: validate and sanitise all request-derived input.
🧰 Tools
🪛 Luacheck (1.2.0)
[warning] 76-76: shadowing definition of variable 'err' on line 62
(W421)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@matrix/matrix.lua` around lines 70 - 85, Sanitise and flatten request-derived
inputs before concatenating into data["formatted_body"] and data["body"]: run
request_method, remote_addr, request_host, self.ctx.bw.uri, reason, reason_data
and every header name/value through an HTML-escape helper (e.g., escape_html)
for formatted_body and tostring for plain body, and when reading headers from
ngx_req.get_headers() handle table values by flattening with table.concat(v, ",
") (or tostring on non-tables) so duplicate headers do not cause concatenation
errors; apply this transformation where data["formatted_body"] and data["body"]
are built and when iterating headers returned by ngx_req.get_headers().
| if self.variables["MATRIX_INCLUDE_HEADERS"] == "yes" then | ||
| local headers, err = ngx_req.get_headers() | ||
| if not headers then | ||
| data["formatted_body"] = data["formatted_body"] .. "error while getting headers: " .. err | ||
| data["body"] = data["body"] .. "\n error while getting headers: " .. err | ||
| else | ||
| data["formatted_body"] = data["formatted_body"] .. "<table><tr><th>Header</th><th>Value</th></tr>" | ||
| data["body"] = data["body"] .. "\n\n" | ||
| for header, value in pairs(headers) do | ||
| data["formatted_body"] = data["formatted_body"] .. "<tr><td>" .. header .. "</td><td>" .. value .. "</td></tr>" | ||
| data["body"] = data["body"] .. header .. ": " .. value .. "\n" |
There was a problem hiding this comment.
Do not forward raw request headers to Matrix.
With MATRIX_INCLUDE_HEADERS=yes, this sends Authorization, Cookie, CSRF tokens, and similar secrets into a third-party room. Switch to an allow-list or redact sensitive header names before serialising them.
As per coding guidelines, Never log request bodies, cookies, bearer tokens, webhook secrets, or API keys.
🧰 Tools
🪛 Luacheck (1.2.0)
[warning] 76-76: shadowing definition of variable 'err' on line 62
(W421)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@matrix/matrix.lua` around lines 75 - 85, When MATRIX_INCLUDE_HEADERS is
enabled in matrix.lua, avoid forwarding raw headers in the loop that appends to
data["formatted_body"] and data["body"]; instead implement an allow-list (e.g.,
only include common safe headers like "User-Agent", "Accept", "Host") or
explicitly redact sensitive names (e.g., "Authorization", "Cookie",
"Set-Cookie", "X-CSRF-Token", "Proxy-Authorization", "Authentication") before
serialising, and in the for header,value in pairs(headers) block replace
sensitive values with a placeholder like "[REDACTED]" (or skip adding those
headers entirely) so no bearer tokens, cookies, CSRF tokens, or secrets are sent
to Matrix.
| local httpc, err = http_new() | ||
| if not httpc then | ||
| self.logger:log(ERR, "can't instantiate http object : " .. err) | ||
| end | ||
| -- Prapare data | ||
| local base_url = self.variables["MATRIX_BASE_URL"] | ||
| local access_token = self.variables["MATRIX_ACCESS_TOKEN"] | ||
| local room_id = self.variables["MATRIX_ROOM_ID"] | ||
| local txn_id = tostring(os.time()) | ||
| local url = string.format("%s/_matrix/client/r0/rooms/%s/send/m.room.message/%s", base_url, room_id, txn_id) | ||
| local message_data = { | ||
| msgtype = "m.text", | ||
| body = data["body"], | ||
| format = "org.matrix.custom.html", | ||
| formatted_body = data["formatted_body"] | ||
| } | ||
| local post_data = cjson.encode(message_data) | ||
| -- Send request | ||
| local res, err_http = httpc:request_uri(url, { | ||
| method = "PUT", | ||
| body = post_data, | ||
| headers = { | ||
| ["Content-Type"] = "application/json", | ||
| ["Authorization"] = "Bearer " .. access_token -- Access Token im Header | ||
| } | ||
| }) | ||
| httpc:close() | ||
| if not res then | ||
| self.logger:log(ERR, "error while sending request : " .. err_http) | ||
| end | ||
| if res.status < 200 or res.status > 299 then |
There was a problem hiding this comment.
Return immediately after HTTP client/request failures.
In both send paths, if http_new() or request_uri() fails, the code still calls httpc:close() and dereferences res.status. That turns an upstream failure into a Lua exception instead of a clean error path. Guard the nil branches and return before touching httpc or res.
Also applies to: 185-203
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@matrix/matrix.lua` around lines 106 - 136, The send path currently continues
after http_new() or httpc:request_uri() failures, which causes httpc:close() and
res.status dereferences on nil; update the logic in the Matrix send function(s)
to immediately return an error when http_new() fails (http_new -> httpc nil) and
likewise after request_uri() fails (res nil), ensuring you do not call
httpc:close() or access res.status in those failure branches; specifically, in
the code using http_new(), httpc, httpc:request_uri, httpc:close and res.status
(also apply the same fix to the second send path around the block referenced at
185-203), log the error and return early before touching httpc or res.
| local base_url = self.variables["MATRIX_BASE_URL"] | ||
| local access_token = self.variables["MATRIX_ACCESS_TOKEN"] | ||
| local room_id = self.variables["MATRIX_ROOM_ID"] | ||
| local txn_id = tostring(os.time()) | ||
| local url = string.format("%s/_matrix/client/r0/rooms/%s/send/m.room.message/%s", base_url, room_id, txn_id) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /workspace && find . -name "matrix.lua" -type fRepository: bunkerity/bunkerweb-plugins
Length of output: 131
🏁 Script executed:
head -n 200 matrix/matrix.lua | tail -n +100Repository: bunkerity/bunkerweb-plugins
Length of output: 3351
🌐 Web query:
Matrix client API transaction ID idempotency specification
💡 Result:
The Matrix Client-Server API uses client-generated transaction IDs (txnId) in the URL path of HTTP PUT requests to ensure idempotency. This prevents duplicate processing when clients retry requests due to network issues or timeouts. Key aspects of the specification: - Purpose: Distinguishes new requests from retransmissions, making requests idempotent. - Client behavior: Generate a unique txnId for each new request (recommended: v4 UUIDs or timestamp + monotonically increasing integer). Change txnId after each request completes. - Server behavior: If the txnId and full HTTP path match a previous request, treat as retransmission: return the identical HTTP status code and response body as the original (e.g., 200 OK with the original event_id for room send endpoints). Ignore the duplicate request. - Scope: Per device and per HTTP endpoint. Different endpoints (e.g., /rooms/{roomId}/send/{eventType}/{txnId} vs /sendToDevice/{eventType}/{txnId}) are distinct even with same txnId. Scoped to access token/device: re-login with new device creates new scope; token refresh retains scope. - POST endpoints: Some allow POST without txnId; PUT with txnId strongly recommended where optional. - Changes: Clarified in ~v1.7 (MSC via #1449). Improved txnId recommendations in v1.12 (#1888). Example endpoint: PUT /_matrix/client/v3/rooms/{roomId}/send/{eventType}/{txnId} This applies to state events, room messages, send-to-device, etc. No separate /transactions endpoint for client-server; that's server-server.
Citations:
- 1: Clarification to transaction identifier idempotent semantics matrix-org/matrix-spec#1449
- 2: https://spec.matrix.org/v1.9/client-server-api/
- 3: https://spec.matrix.org/v1.14/client-server-api/
- 4: https://spec.matrix.org/v1.15/client-server-api/
- 5: https://spec.matrix.org/v1.10/client-server-api
- 6: https://spec.matrix.org/latest/client-server-api/
- 7: Improve recommendation for how to form transaction IDs matrix-org/matrix-spec#1888
Use a collision-resistant transaction ID for Matrix idempotency keys.
The Matrix Client-Server API treats transaction IDs (txnId) as idempotency keys: when the server receives a request with the same txnId and endpoint path, it returns the cached response (same status code and body) rather than reprocessing. Using os.time() at lines 114 and 178 only provides second-level precision—multiple requests sent within the same second will share an identical txn_id, causing the Matrix server to collapse them into a single message.
Generate a unique ID per send instead. Options include:
- Concatenate
os.time()withngx.var.msecfor millisecond precision (if available) - Use
ngx.var.request_idif per-request uniqueness suffices - Implement a monotonic counter within the plugin state (with careful locking for concurrent worker access)
Lines 114 and 178 both require fixes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@matrix/matrix.lua` around lines 111 - 115, The txn_id currently set via
tostring(os.time()) is not collision-resistant; replace it with a per-send
unique id and update both places where txn_id is generated/used (the variable
named txn_id in matrix.lua). For example, build txn_id by concatenating
os.time() with a higher-resolution or unique per-request value: use ngx.var.msec
if available (tostring(os.time()) .. "-" .. ngx.var.msec), or ngx.var.request_id
when present, or combine ngx.now()*1000 with a short random suffix to guarantee
uniqueness across rapid successive sends; ensure both locations that set txn_id
use the same new generation logic so Matrix idempotency keys are unique per
send.
| # Table of contents | ||
|
|
||
| - [Matrix Notification Plugin](#matrix-notification-plugin) | ||
| - [Table of contents](#table-of-contents) | ||
| - [Prerequisites](#prerequisites) | ||
| - [Setup](#setup) | ||
| - [Docker](#docker) | ||
| - [Swarm](#swarm) | ||
| - [Kubernetes](#kubernetes) | ||
| - [Settings](#settings) | ||
| - [TODO](#todo) |
There was a problem hiding this comment.
Remove the stale TODO entry from the table of contents.
Lines 5-15 link to #todo, but the document has no matching section.
As per coding guidelines, Documentation should be concise, accurate, and written in British English: Keep the structure clear with a sensible heading hierarchy.
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 15-15: Link fragments should be valid
(MD051, link-fragments)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@matrix/README.md` around lines 5 - 15, The Table of contents contains a stale
entry linking to "#todo" that does not exist; open the README.md section
containing the bulleted list (the lines under "Table of contents") and remove
the "- [TODO](`#todo`)" list item so the TOC matches the actual headings and keeps
the documentation concise and accurate.
| ## Docker | ||
|
|
||
| ```yaml | ||
| version: '3' | ||
|
|
||
| services: | ||
|
|
||
| bunkerweb: | ||
| image: bunkerity/bunkerweb:1.5.10 | ||
| ... | ||
| environment: | ||
| - USE_MATRIX=yes | ||
| - MATRIX_BASE_URL=https://matrix.org | ||
| - MATRIX_ROOM_ID=!yourRoomID:matrix.org | ||
| - MATRIX_ACCESS_TOKEN=your-access-token | ||
| ... | ||
| ``` |
There was a problem hiding this comment.
Update the examples to the 1.6 image series.
Lines 42 and 60 still pin bunkerity/bunkerweb:1.5.10, but this PR is explicitly about 1.6 compatibility. The examples should not steer users onto the wrong runtime version.
As per coding guidelines, Prefer concrete instructions, accurate examples, and explicit prerequisites.
Also applies to: 52-68
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@matrix/README.md` around lines 34 - 50, Update the Docker examples in
matrix/README.md to reference the 1.6 image series: replace all occurrences of
the image tag "bunkerity/bunkerweb:1.5.10" (seen in the bunkerweb service
blocks) with the appropriate 1.6 tag (e.g., "bunkerity/bunkerweb:1.6" or the
exact 1.6.x release you intend to document) so examples match this PR's 1.6
compatibility; ensure both instances mentioned in the diff are updated.
| | Setting | Default | Context | Multiple | Description | | ||
| | -------------------- | ---------------------------- | --------- | -------- | --------------------------------------------------------------------------------------------------- | | ||
| | `USE_MATRIX` | `no` | multisite | no | Enable sending alerts to a Matrix room. | | ||
| | `MATRIX_BASE_URL` | `https://matrix.org` | global | no | Base URL of the Matrix server. | | ||
| | `MATRIX_ROOM_ID` | `!yourRoomID:matrix.org` | global | no | Room ID of the Matrix room to send notifications to. | | ||
| | `MATRIX_ACCESS_TOKEN` | ` ` | global | no | Access token to authenticate with the Matrix server. | | ||
| | `MATRIX_ANONYMIZE_IP` | `no` | global | no | Mask the IP address in notifications. | | ||
| | `MATRIX_INCLUDE_HEADERS` | `no` | global | no | Include request headers in notifications. | |
There was a problem hiding this comment.
Regenerate the settings table from plugin.json.
This table already drifts from matrix/plugin.json — for example, MATRIX_ACCESS_TOKEN is shown as a space here, but its default is an empty string in matrix/plugin.json. Please rerun .tests/misc/json2md.py instead of maintaining the table by hand.
As per coding guidelines, Each plugin README contains a settings table generated from plugin.json via .tests/misc/json2md.py; regenerate it whenever settings change rather than hand-editing the table.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@matrix/README.md` around lines 86 - 93, The settings table in README.md has
drifted from matrix/plugin.json; rerun the JSON-to-Markdown generator
(.tests/misc/json2md.py) against matrix/plugin.json to regenerate the table (so
defaults like MATRIX_ACCESS_TOKEN show as an empty string), replace the manually
edited table in README.md with the generated output, and commit the regenerated
README instead of hand-editing the table.
| except BaseException as e: | ||
| logger.debug(format_exc()) | ||
| logger.error(f"Failed to get matrix ping: {e}") | ||
| ret["error"] = str(e) |
There was a problem hiding this comment.
Do not expose raw exceptions in the card payload.
Line 21 returns str(e) to the UI. Errors from the ping path can include internal URLs or other sensitive details, and this payload is rendered. Keep the full exception in server-side logs and return a generic UI error marker instead.
As per coding guidelines, Never expose secrets, webhook URLs, API keys, or tokens in the returned card payload — they are rendered into the UI.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@matrix/ui/actions.py` around lines 18 - 21, The code assigns str(e) to
ret["error"], exposing potentially sensitive details; change this so the full
exception remains only in server logs (keep logger.debug(format_exc()) and
logger.error(f"Failed to get matrix ping: {e}") for diagnostics) but replace
ret["error"] with a generic, non-sensitive marker such as "Unable to retrieve
matrix ping" or "internal_error" (update the assignment to ret["error"]
accordingly) — locate the try/except handling around the ping path where
ret["error"] is set and remove any raw exception string from the payload.
| def matrix(**kwargs): | ||
| pass No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the matrix plugin directory and examine ui/actions.py
find . -path "*matrix/ui/actions.py" -type fRepository: bunkerity/bunkerweb-plugins
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Read the matrix ui/actions.py file
if [ -f "./matrix/ui/actions.py" ]; then
wc -l ./matrix/ui/actions.py
fiRepository: bunkerity/bunkerweb-plugins
Length of output: 96
🏁 Script executed:
#!/bin/bash
# Display the file contents around lines 29-30
if [ -f "./matrix/ui/actions.py" ]; then
cat -n ./matrix/ui/actions.py | sed -n '20,40p'
fiRepository: bunkerity/bunkerweb-plugins
Length of output: 335
🏁 Script executed:
#!/bin/bash
# Search for calls to the matrix() function to see what callers expect
rg "matrix\(" --type py -B 2 -A 2Repository: bunkerity/bunkerweb-plugins
Length of output: 190
🏁 Script executed:
#!/bin/bash
# Look at other plugin ui/actions.py files to understand the pattern
find . -path "*/ui/actions.py" -type f | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && head -40 {}'Repository: bunkerity/bunkerweb-plugins
Length of output: 3687
Return an empty dict from matrix().
Line 29-30: The matrix() function currently returns None implicitly, breaking the expected contract that all plugin main-page entry points return a dictionary. Other plugins (clamav, coraza, crowdsec, discord) consistently implement this pattern. Return {} at minimum to match the standard.
Suggested fix
def matrix(**kwargs):
- pass
+ return {}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def matrix(**kwargs): | |
| pass | |
| def matrix(**kwargs): | |
| return {} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@matrix/ui/actions.py` around lines 29 - 30, The function matrix(**kwargs)
currently returns None which violates the plugin main-page contract; update the
matrix function to return an empty dict ({}). Locate the matrix function
definition and replace the pass/implicit None with a return {} so it matches
other plugin entry points (e.g., clamav, coraza, crowdsec, discord) and ensures
callers receive a dictionary.
2 participants
Initially made this for myself, maybe you want to add it to your collection.
Code is heavily inspired by your Webhook plugin.
This is how it looks in Element.
