Merge pull request #1807 from netalertx/next_release

Next release
This commit is contained in:
Jokob @NetAlertX authored and GitHub committed 2026-09-25 08:46:22 +10:00
commit eea3ac88c0
36 files changed
+1979 -363

No files matched your search

+5 -1
View File
@@ -48,6 +48,8 @@ plugin_objects.write_result_file() # exactly once, at the end
Full column spec: `docs/PLUGINS_DEV_DATA_CONTRACT.md`. Note `helpVal1-4`/`watchedValue1-4` both preserve a real `0`/`False` you pass explicitly — only an omitted (`None`) value defaults to `""`.
Every mapped field (`objectPrimaryId`/`objectSecondaryId`/`watchedValue1-4`/`extra`/`helpVal1-4`) is HTML/control-char-stripped by default before it's persisted: plugin output is untrusted (network responses, device-reported names, etc.). `foreignKey` is always sanitized too, unconditionally. Only opt a column out (`"allow_raw_text": true`) if it's a display-only type (`textarea_readonly`); see `docs/PLUGINS_DEV.md#field-sanitization`.
## Execution Phases
| Phase | Trigger |
@@ -71,7 +73,9 @@ Full column spec: `docs/PLUGINS_DEV_DATA_CONTRACT.md`. Note `helpVal1-4`/`watche
## Before Opening a PR
Check the plugin against the [Conventions Checklist](../../../docs/PLUGINS_DEV.md#conventions-checklist) — `RUN` default, schedule precedent, `RUN_TIMEOUT` semantics, reusing core settings instead of duplicating them, description length (renders in the Settings UI — keep it short), and the multi-instance settings pattern (nested array + popup-form, see `rest_import`, not a hardcoded "primary"/"secondary" pair). Most plugin PR review comments trace back to one of these, and `test/plugins/test_plugin_conventions.py` mechanically enforces the RUN-default, description-length, hardcoded-default-drift, RUN_TIMEOUT-reuse-in-loop, and array/object dataType-default_value-mismatch items — run it after touching a plugin.
Check the plugin against the [Conventions Checklist](../../../docs/PLUGINS_DEV.md#conventions-checklist) — `RUN` default, schedule precedent, `RUN_TIMEOUT` semantics, reusing core settings instead of duplicating them, description length (renders in the Settings UI — keep it short), the multi-instance settings pattern (nested array + popup-form, see `rest_import`, not a hardcoded "primary"/"secondary" pair), and `allow_raw_text` restricted to display-only column types. Most plugin PR review comments trace back to one of these, and `test/plugins/test_plugin_conventions.py` mechanically enforces the RUN-default, description-length, hardcoded-default-drift, RUN_TIMEOUT-reuse-in-loop, array/object dataType-default_value-mismatch, and allow_raw_text-type-restriction items — run it after touching a plugin.
If the plugin needs a new system package or Python dependency, mirroring it into the root `Dockerfile`/`requirements.txt` alone is not enough: see the Conventions Checklist's build-target-mirroring bullet for `.devcontainer/Dockerfile` (regenerate via `.devcontainer/scripts/generate-configs.sh`, don't hand-edit it), `Dockerfile.debian`, and `install/ubuntu24`/`install/proxmox`'s own `requirements.txt` files.
## Starting Point
+25 -3
View File
@@ -1,6 +1,6 @@
---
name: plugin-review
description: Read when reviewing a plugin PR or auditing an existing plugin script (server/plugins/*/script.py or equivalent). Covers the one check not already mechanically enforced by test_plugin_conventions.py - plugin scripts embedding their own raw SQL instead of an existing/new model method - plus a worked real-PR example.
description: Read when reviewing a plugin PR or auditing an existing plugin script (server/plugins/*/script.py or equivalent). Covers three checks not already mechanically enforced by test_plugin_conventions.py - plugin scripts embedding their own raw SQL instead of an existing/new model method, suspicious/attacker-influenced plugin data not being logged, and a new dependency not reaching every build target - plus worked real-PR examples.
---
# Plugin Review
@@ -20,8 +20,8 @@ Plugin scripts write their results to `RESULT_FILE` via `plugin_helper.Plugin_Ob
## Review flow for a raw SQL query in a plugin
1. **Does an existing model method already do this?** Check the relevant `server/models/*_instance.py` file (`DeviceInstance`, `EventInstance`, `PluginObjectInstance`, etc.) before assuming one needs to be added.
2. **If not, is it worth adding one** (`server/models/device_instance.py` etc.), or is this genuinely a one-off maintenance/schema query that belongs in the core-plugin exception list above?
3. **Check the collation the query relies on** against the column's actual schema (`server/db/schema/app.sql`) rather than assuming — `devMac`/`eveMac`/`sesMac`/`scanMac`/`devParentMAC` are declared `COLLATE NOCASE` at the column level, so an explicit `COLLATE NOCASE` against one of them in a new query is redundant (harmless, but a sign the author didn't check). `devName` has **no** column-level collation — an explicit `COLLATE NOCASE` there is genuinely necessary if case-insensitive name matching is intended, not a mistake.
2. **If not, is it worth adding one** (`server/models/device_instance.py` etc.), or is this a one-off maintenance/schema query that belongs in the core-plugin exception list above?
3. **Check the collation the query relies on** against the column's actual schema (`server/db/schema/app.sql`) rather than assuming — `devMac`/`eveMac`/`sesMac`/`scanMac`/`devParentMAC` are declared `COLLATE NOCASE` at the column level, so an explicit `COLLATE NOCASE` against one of them in a new query is redundant (harmless, but a sign the author didn't check). `devName` has **no** column-level collation — an explicit `COLLATE NOCASE` there is necessary if case-insensitive name matching is intended, not a mistake.
4. **Parameterization** — `?` placeholders, never string-formatted values into the query (this part is usually already fine; flag it if not).
## Worked example: PR #1788 (DOCKERDISC plugin)
@@ -32,3 +32,25 @@ Two raw queries in `server/plugins/dockerdisc/script.py`:
- `resolve_host_mac()`: `SELECT devMac FROM Devices WHERE devName = ? COLLATE NOCASE` — a name lookup with real 0/1/many-match handling (falls back to a manually-configured MAC on ambiguity or no match). No existing method covers this. `devName` has no column-level collation, so the explicit `COLLATE NOCASE` here is correct, not redundant. **Fix: add `DeviceInstance.getAllByName(name)` returning every match** (not just one — the plugin's own ambiguity detection needs the full set), and have the plugin call that instead.
This is the shape of the fix in general: an existence/single-row check usually already has a model method; a query with plugin-specific result handling (ambiguity, filtering) usually needs a small new method added rather than a workaround in the plugin itself.
## The second check this skill adds: suspicious/attacker-influenced plugin data must be logged
A plugin that parses data from an unauthenticated peer (DHCP options, mDNS/Avahi records, NetBIOS name-service responses, SSDP/UPnP, any broadcast/discovery protocol) is trusting the network, not the device it's nominally scanning — any device on the segment can answer. When such a value gets rejected, sanitized, or otherwise flagged as suspicious/malformed, that's a security-relevant event, not routine parsing noise: **is it logged?**
At minimum, every such detection needs `mylog("none", ...)` (`logger.py`'s `debugLevels` — `"none"` is level 0, the always-shown floor, not filtered out at any configured `LOG_LEVEL` — matching how this codebase already logs real errors, e.g. `mylog("none", f"[Plugins] ⚠ ERROR: {e}")`). A silently-dropped or silently-mangled value with no log trace is the finding to raise — an admin investigating "why does this device's name look wrong" or "was my network probed" has nothing to go on otherwise.
**A user-facing alert (`write_notification()`, `server/messaging/in_app.py`) is a separate, materially bigger decision: don't require it as a blocking condition the way the log line is.** It's persistent and unprompted, and (per existing precedent: `api_server_start.py`'s unauthorized-access-attempt alert fires unconditionally, with no rate-limiting anywhere in this codebase) a repeat offender re-sending the same payload every scan cycle can spam it indefinitely unless the PR gates it correctly on `process_plugin_events()`'s existing per-object status (`server/plugin.py:769-791`): fire only on `"new"` or `"watched-changed"`, never on `"watched-not-changed"` (that status is set every cycle a value stays the same, so alerting on it defeats the suppression entirely). For a missing-object alert, fire only on the transition into `"missing-in-last-scan"` (`server/plugin.py`'s `if tmpObj.status != "missing-in-last-scan":` guard around line 807), not on every cycle the object remains in that status. If a PR adds `write_notification()` for this without matching that gating, that's the thing to flag, not the absence of a user-facing alert on its own.
**Worked example (generalized):** a plugin parses an unauthenticated broadcast-protocol response (e.g. a DHCP option, an mDNS/NetBIOS record) and copies a field from it verbatim into a stored value with no validation. The fix centralizes both the sanitization *and* the `mylog("none", ...)` call in one shared, plugin-agnostic enforcement point (`plugin_object_class.__init__`, `server/plugin.py`) rather than leaving individual plugin authors to remember either — the same reasoning as the raw-SQL check above: a check that depends on every plugin author independently thinking to add it will eventually ship without it.
## The third check this skill adds: a new dependency has to reach every build target the plugin should run on
If a PR adds a system package (`apk add` in the root `Dockerfile`) or a Python dependency (`requirements.txt`), check whether it actually reached every place that needs it, not just the one file the diff touched:
- **`.devcontainer/Dockerfile`** is a committed, git-tracked file generated by concatenating the root `Dockerfile` with `.devcontainer/resources/devcontainer-Dockerfile` (`.devcontainer/scripts/generate-configs.sh`). It is not read fresh from the root `Dockerfile` at build time, so a PR that edits the root `Dockerfile` without re-running that script leaves this file stale, silently missing the new package for anyone testing inside the devcontainer. CI enforces this: the `check-devcontainer-dockerfile` job (`.github/workflows/code-checks.yml`) regenerates the file and fails the build if it doesn't match the committed one.
- **`Dockerfile.debian`** (`docs/BUILDS.md`) is a second, separately-maintained build target with its own `apt-get install` list and `setcap` calls. A system package (and its capability grant, if the plugin needs one, like `arp-scan`/`nmap`/`iw`) added only to the Alpine `Dockerfile` leaves this target broken.
- **`install/ubuntu24/requirements.txt`** and **`install/proxmox/requirements.txt`** are separate Python dependency lists for their own non-Docker install methods, not mechanically synced with the root `requirements.txt` (they already drift from it today) - check whether the new dependency is actually needed by those install paths too.
For the latter two, `scripts/check_dependency_mirroring.py` (wired into the non-blocking `check-dependency-mirroring` CI job) flags a PR that touches `Dockerfile` without `Dockerfile.debian`, or `requirements.txt` without both `install/*` copies - it only knows the sibling file wasn't touched at all, not whether the specific package was actually needed there, so treat a flag as a prompt to check, not a verdict.
A worked example: a WiFi-scanning plugin PR added `iw` plus its `setcap` grant to the root `Dockerfile` only. The devcontainer's checked-in Dockerfile went stale (missing `iw` until someone regenerates it), and `Dockerfile.debian` never got `iw` or a `setcap` line for it at all - the plugin silently can't scan on either target, caught only because the plugin's own error handling logs "not found" rather than crashing.
+7 -6
View File
@@ -22,20 +22,21 @@ Both are plausible, well-written, and wrong. Reading the code first catches both
1. **Understand the current mechanism by reading the actual code before writing anything.** Cite `file:line` for every claim about current behavior. Delegate to an Explore/general-purpose agent for breadth if the surface area is large, but treat its findings as a starting point to spot-check, not a finished citation — verify anything load-bearing yourself before it goes in the PRD. The same applies to a prior audit or PRD this one continues from: re-read its full detail section for the specific finding, not just a one-line summary-table row, before citing or extending it — a summary row can omit a caveat ("already indexed," "already fixed elsewhere") that only the detail text states, and citing the row alone can reintroduce a claim the detail text already corrected.
2. **Challenge the idea before designing it.** If the user proposes a solution, ask: is this solving the right problem? Does it conflate unrelated concerns (see axis-separation, next)? Does a similar or previously-rejected mechanism already exist that this would collide with semantically? A naming near-collision with an existing field/concept that has different, incompatible semantics is a signal to stop and check precedence rules, not a coincidence to wave off.
3. **Identify the independent axes.** A feature request that arrives as "option A and option B" is often two or three orthogonal concerns bundled together — e.g. "should this exist at all," "should it notify," and "should it assert presence" are three separate questions, not one. Cramming them into a single enum/flag produces combinations you can't express later (what if a plugin wants A+C but not B?). Give each axis its own mechanism.
3. **Identify the independent axes.** A feature request that arrives as "option A and option B" is often two or three orthogonal concerns bundled together — e.g. "should this exist at all," "should it notify," and "should it assert presence" are three separate questions, not one. Cramming them into a single enum/flag produces combinations you can't express later (what if a plugin wants A+C but not B?). Give each axis its own mechanism. A recurring instance of this: if the design detects or handles data that indicates a likely security-relevant event (a plugin sanitizing suspicious/attacker-influenced input, a rejected/malformed value, an auth failure, etc.), "log it" and "alert the user about it" are two separate axes, not one. Server-log visibility (`mylog("none", ...)` in this codebase — the always-shown floor, matching how real errors are already logged) is close to a hard requirement whenever such an event is detected at all; a user-facing alert (`write_notification()`) is a materially bigger decision — it's persistent, unprompted, and (per existing precedent, e.g. `api_server_start.py`'s unauthorized-access alert) not rate-limited anywhere in this codebase, so a repeat offender can spam it. Don't let "should we detect/handle this" and "should we alert the user about it" collapse into one decision — record the logging as close to a given, and the user-facing alert as its own, explicit, separately-decidable (and separately deferrable) open issue.
4. **For every mechanism, trace every downstream consumer — not just the first one you find.** The single highest-value question before calling a design complete: "where else does this exact same check or logic get independently re-derived?" In a codebase without one source of truth for a concept (e.g. "is this record currently active" computed by three different queries in three different files), patching the first occurrence and stopping is the most common way a design ships with a hidden, silent gap. Grep for the pattern, not just the function you already know about.
5. **Record rejected alternatives with the reasoning, not just the chosen design.** Give it its own subsection (`### Rejected: X`). Without this, a future reader — or your own future self — re-proposes the rejected idea because the "why not" only ever existed in a conversation, not in the document.
6. **Force every open question to an explicit decision**, even if the decision is "accept as-is for v1, revisit if feedback says otherwise." An open question left unresolved in a PRD gets silently decided by whoever implements it — usually differently than anyone actually intended.
6. **Force every open question to an explicit decision**, even if the decision is "accept as-is for v1, revisit if feedback says otherwise." An open question left unresolved in a PRD gets silently decided by whoever implements it — usually differently than anyone intended.
7. **Write the test plan as part of the PRD, not after.** Concrete test cases — naming real functions/queries, not "add tests for X" — force you to notice design gaps you'd otherwise miss; the moment you try to write "assert Y happens" and realize the current design can't produce Y is often the first time the gap becomes visible. Check the repo for an existing test pattern for this shape of change before inventing a new one (e.g. a prior presence-logic bug fixed via `test/db_test_helpers.py` fixtures is the template for the next one, not a reason to build new test infrastructure).
8. **Ask explicitly whether validating this needs real end-to-end infrastructure** (a new or modified plugin, a UI click-through) or whether synthetic unit-level fixtures suffice — don't assume either way. Check whether the functions under test take a DB connection/dict/list as a parameter (testable in isolation, no real plugin needed) or require a real file on disk (harder to fake, may need one).
9. **Check performance against the real schema and real scale, not assumptions.** For every new or changed query: does it use an existing index, or add an unindexed lookup, a new join, or a correlated subquery? Grep `CREATE INDEX` for *every* index on the tables involved, not just the first one you find — a column can have both a plain index and a separate expression index (e.g. `idx_eve_mac_date_type ON Events(eveMac, ...)` alongside `idx_eve_lower_mac_date_type ON Events(LOWER(eveMac), ...)`), and missing the second one produces a wrong verdict. Then weigh cost by how often the query runs (once is nothing; every few minutes forever is a standing cost) and by real scale — **production users run 10,000+ devices**, not a homelab handful. A `CurrentScan` with 2-5 rows per device (one per contributing plugin) is routinely 20,000-50,000+ rows in one cycle; reason about that number, not a smaller hopeful one. A correlated `EXISTS`/subquery re-evaluated per outer row is fine *if* the correlated column is indexed — e.g. `current_scan_presence_condition()` (`server/scan/presence.py`) is exactly this shape against `CurrentScan.scanMac`, covered by `idx_currentscan_scanmac`. The real risk is an unindexed correlated lookup: an accidental self-join scanning the full inner table per outer row, which looks fine and passes tests at small scale but isn't at production scale. Check with `EXPLAIN QUERY PLAN` at a realistic row count, built against the *complete* real index set (copy every `CREATE INDEX` for the table, or run it against an actual `app.db`) rather than a hand-picked subset — a partial index set produces a misleading plan in either direction, not just "looks worse than it is." If it comes back unindexed for real, a `GROUP BY` aggregate is the usual fix.
10. **Do a dedicated final-check pass, out loud, before calling it done.** Re-read the whole document end to end and specifically check:
- Did a correction made mid-document actually propagate everywhere it needed to (the Design subsection *and* Affected Files *and* Tests *and* any execution-plan summary)? A correction landing in one place and not its siblings is worse than never catching it, because now the document silently contradicts itself.
- Did a correction made mid-document propagate everywhere it needed to (the Design subsection *and* Affected Files *and* Tests *and* any execution-plan summary)? A correction landing in one place and not its siblings is worse than never catching it, because now the document silently contradicts itself.
- Does every "this is the cleanest/simplest real case" claim still hold up if you actually re-read that specific piece of code right now, or was it asserted by pattern-matching a name/category? Re-verify, don't re-assert.
- Does anything render incorrectly as markdown — an unfenced ASCII diagram or code block will collapse into one line under lazy-paragraph-continuation, the same class of bug as a list missing its preceding blank line.
- Do any internal anchor links' slugs actually match their headings?
- Do any internal anchor links' slugs match their headings?
- Does the design still cleanly separate its axes, or did a later addition quietly re-conflate two concerns inside what's supposed to be a single-purpose mechanism (the same mistake step 3 exists to catch at the top level can reappear one level down inside an individual mechanism's own value set — e.g. a 3-value enum where two of the values are secretly independent booleans in a trenchcoat).
11. **Leave a visible trail of corrections instead of silently rewriting.** When a review pass — yours or someone else's — finds something wrong, write "**Correction (caught in review):** ..." inline rather than quietly fixing the earlier text and moving on. This is what makes a PRD trustworthy to a second reader: they can see what was checked and what changed, not just receive a polished final answer with no visible seams.
12. **When marking a PRD Implemented, ask once whether anything found along the way generalizes beyond this one document** — a tooling trick, a wrong assumption that took real digging to disprove, a review catch that would recur on the next PRD. The PRD's own "Implementation notes" section is the right place for what happened *in this PRD*; a skill or memory update is the right place for anything that would otherwise have to be rediscovered next time. One question, not a mandatory new section — skip it when nothing generalizes, don't force it on every PRD regardless of size.
## Structure to follow
@@ -45,7 +46,7 @@ Both are plausible, well-written, and wrong. Reading the code first catches both
- **Open issues** — each with an explicit recorded decision (step 6), not left dangling.
- **Affected files** — concrete `file:function:line` references, not bare filenames.
- **Backward compatibility** — explicit default values and why they preserve current behavior for existing consumers.
- **Performance impact** — the baseline (what's unindexed/slow *today*, independent of this change), what the change adds that's negligible, what's genuinely new and worth mitigating, and concrete mitigations rather than a vague "should be fine" (step 9).
- **Performance impact** — the baseline (what's unindexed/slow *today*, independent of this change), what the change adds that's negligible, what's new and worth mitigating, and concrete mitigations rather than a vague "should be fine" (step 9).
- **Docs/skills to update** — anywhere this needs to be reflected outside the code itself (external docs, paired skill files, template files new authors copy from).
- **Tests** — organized by mechanism, each case naming the real function/query it exercises and the concrete assertion (step 7), plus a manual verification checklist for anything that can't be unit-tested (including an `EXPLAIN QUERY PLAN` check at realistic scale if the Performance impact section found a genuine risk).
- **(Optional) Execution plan** — phased, referencing the same file/function names used above rather than restating the design in vaguer terms.
@@ -56,4 +57,4 @@ If a skill already documents the subsystem the feature touches, load it before r
## Where to save
`.gemini/internal-docs/PRDs/<kebab-case-name>.md`, unless the user specifies otherwise. Mark the status line (`**Status:** Draft — pending review`) so it's clear this hasn't been approved yet, and keep the author line accurate about who actually made the calls (a design discussion with an assistant is not sole assistant authorship).
`.gemini/internal-docs/PRDs/<kebab-case-name>.md`, unless the user specifies otherwise. Mark the status line (`**Status:** Draft — pending review`) so it's clear this hasn't been approved yet, and keep the author line accurate about who made the calls (a design discussion with an assistant is not sole assistant authorship).
+1 -1
View File
@@ -34,7 +34,7 @@ Covers what happens after a plugin's rows land in `CurrentScan`: presence comput
6. `update_presence_from_CurrentScan(db)` — sets `devPresentLastScan` from `CurrentScan` for this cycle (step 2 reads this as "previous" on the *next* cycle).
7. `update_devPresentLastScan_based_on_nics(db)` — NIC/parent-child presence aggregation; can override step 6 for parent devices.
8. `update_devPresentLastScan_based_on_force_status(db)` — the user's manual `devForceStatus` override; runs last, wins over everything above.
9. `update_vendors_from_mac`, `update_ipv4_ipv6`, `update_icons_and_types` — `update_ipv4_ipv6()` does **not** go through `LatestDeviceScan`/`FIELD_SPECS`; it reads `CurrentScan` directly with its own `PARTITION BY scanMac, address_family` ranking, so a device reporting both an IPv4 and an IPv6 row in the same cycle gets both `devPrimaryIPv4`/`devPrimaryIPv6` set from that cycle, not just whichever family happened to win `devLastIP`'s single-value reduction. See `.gemini/internal-docs/PRDs/dual-stack-primary-ip-support.md`.
9. `update_vendors_from_mac`, `update_ipv4_ipv6`, `update_icons_and_types` — `update_ipv4_ipv6()` does **not** go through `LatestDeviceScan`/`FIELD_SPECS`; it reads `CurrentScan` directly with its own `PARTITION BY scanMac, address_family` ranking, so a device reporting both an IPv4 and an IPv6 row in the same cycle gets both `devPrimaryIPv4`/`devPrimaryIPv6` set from that cycle, not just whichever family happened to win `devLastIP`'s single-value reduction.
10. `pair_sessions_events(db)` — pairs `Events` rows as described above.
11. `create_sessions_snapshot(db)` — `DELETE FROM Sessions; INSERT INTO Sessions SELECT * FROM Convert_Events_to_Sessions`. `Sessions` reflects step 10's pairing from here.
12. `insertOnlineHistory(db)` — dashboard graph rollup.
+7 -2
View File
@@ -27,7 +27,11 @@ Cut anything that narrates the past instead of stating the present:
## Rule 2: plain words, fewer words
If a shorter or simpler phrasing says the same thing, use it. Cut qualifiers that don't change the meaning ("actually," "really," "genuinely," "in this exact process"). Prefer a plain verb over a nominalization. A dense skill with real information beats a padded one — trim narration and hedging before trimming facts.
If a shorter or simpler phrasing says the same thing, use it. Cut qualifiers that don't change the meaning ("actually," "really," "genuinely," "in this exact process"). Prefer a plain verb over a nominalization. A dense skill with real information beats a padded one: trim narration and hedging before trimming facts.
## Rule 3: no em-dashes
Never use an em-dash ("—"), in a skill or anywhere else this session writes prose (docs, code comments, PRDs, commit messages, chat replies). Use a period, comma, colon, semicolon, or parentheses instead, whichever actually fits the sentence.
## Sweep before calling a skill clean
@@ -35,9 +39,10 @@ Run this across `.claude/skills/`, `.gemini/skills/`, `.github/skills/` (or a si
```bash
grep -rniE "as of 202|caught in review|caught mid-review|correction:|correction \(|shipped for real|previously|used to be|no longer|originally|was later|historically|in the past|distilled from|real mistake|it turned out|turns out|discovered that|during a past" .claude/skills/ .gemini/skills/ .github/skills/
grep -rn "—" .claude/skills/ .gemini/skills/ .github/skills/
```
Read every hit in context — some are legitimate (a rule instructing PRD authors to write correction trails, or "previously down" describing device state, are not violations). Fix the ones that narrate the skill's own history instead of the system's current behavior.
Read every hit in context: some are legitimate (a rule instructing PRD authors to write correction trails, or "previously down" describing device state, are not violations). Fix the ones that narrate the skill's own history instead of the system's current behavior, and replace every em-dash hit per Rule 3.
## Also applies to: research/audit docs
@@ -60,6 +60,8 @@ plugin_objects.write_result_file() # Exactly once at end
**Important:** The backend processes and deletes the result file almost immediately. Retrieve it quickly if inspecting output.
Every mapped field (`objectPrimaryId`/`objectSecondaryId`/`watchedValue1-4`/`extra`/`helpVal1-4`) is HTML/control-char-stripped by default before it's persisted: plugin output is untrusted (network responses, device-reported names, etc.). `foreignKey` is always sanitized too, unconditionally. Only opt a column out (`"allow_raw_text": true`) if it's a display-only type (`textarea_readonly`); see `docs/PLUGINS_DEV.md#field-sanitization`.
## Execution Phases
| Phase | Trigger |
@@ -83,7 +85,9 @@ plugin_objects.write_result_file() # Exactly once at end
## Before Opening a PR
Check the plugin against the [Conventions Checklist](../../../docs/PLUGINS_DEV.md#conventions-checklist) in `docs/PLUGINS_DEV.md` — `RUN` default, schedule precedent, `RUN_TIMEOUT` semantics, reusing core settings, description length, and the multi-instance settings pattern. Most plugin PR review comments trace back to one of these.
Check the plugin against the [Conventions Checklist](../../../docs/PLUGINS_DEV.md#conventions-checklist) in `docs/PLUGINS_DEV.md` — `RUN` default, schedule precedent, `RUN_TIMEOUT` semantics, reusing core settings, description length, the multi-instance settings pattern, and `allow_raw_text` restricted to display-only column types. Most plugin PR review comments trace back to one of these.
If the plugin needs a new system package or Python dependency, mirroring it into the root `Dockerfile`/`requirements.txt` alone is not enough: see the Conventions Checklist's build-target-mirroring bullet for `.devcontainer/Dockerfile` (regenerate via `.devcontainer/scripts/generate-configs.sh`, don't hand-edit it), `Dockerfile.debian`, and `install/ubuntu24`/`install/proxmox`'s own `requirements.txt` files.
## Starting Point
+25 -3
View File
@@ -1,6 +1,6 @@
---
name: plugin-review
description: Read when reviewing a plugin PR or auditing an existing plugin script (server/plugins/*/script.py or equivalent). Covers the one check not already mechanically enforced by test_plugin_conventions.py - plugin scripts embedding their own raw SQL instead of an existing/new model method - plus a worked real-PR example.
description: Read when reviewing a plugin PR or auditing an existing plugin script (server/plugins/*/script.py or equivalent). Covers three checks not already mechanically enforced by test_plugin_conventions.py - plugin scripts embedding their own raw SQL instead of an existing/new model method, suspicious/attacker-influenced plugin data not being logged, and a new dependency not reaching every build target - plus worked real-PR examples.
---
# Plugin Review
@@ -20,8 +20,8 @@ Plugin scripts write their results to `RESULT_FILE` via `plugin_helper.Plugin_Ob
## Review flow for a raw SQL query in a plugin
1. **Does an existing model method already do this?** Check the relevant `server/models/*_instance.py` file (`DeviceInstance`, `EventInstance`, `PluginObjectInstance`, etc.) before assuming one needs to be added.
2. **If not, is it worth adding one** (`server/models/device_instance.py` etc.), or is this genuinely a one-off maintenance/schema query that belongs in the core-plugin exception list above?
3. **Check the collation the query relies on** against the column's actual schema (`server/db/schema/app.sql`) rather than assuming — `devMac`/`eveMac`/`sesMac`/`scanMac`/`devParentMAC` are declared `COLLATE NOCASE` at the column level, so an explicit `COLLATE NOCASE` against one of them in a new query is redundant (harmless, but a sign the author didn't check). `devName` has **no** column-level collation — an explicit `COLLATE NOCASE` there is genuinely necessary if case-insensitive name matching is intended, not a mistake.
2. **If not, is it worth adding one** (`server/models/device_instance.py` etc.), or is this a one-off maintenance/schema query that belongs in the core-plugin exception list above?
3. **Check the collation the query relies on** against the column's actual schema (`server/db/schema/app.sql`) rather than assuming — `devMac`/`eveMac`/`sesMac`/`scanMac`/`devParentMAC` are declared `COLLATE NOCASE` at the column level, so an explicit `COLLATE NOCASE` against one of them in a new query is redundant (harmless, but a sign the author didn't check). `devName` has **no** column-level collation — an explicit `COLLATE NOCASE` there is necessary if case-insensitive name matching is intended, not a mistake.
4. **Parameterization** — `?` placeholders, never string-formatted values into the query (this part is usually already fine; flag it if not).
## Worked example: PR #1788 (DOCKERDISC plugin)
@@ -32,3 +32,25 @@ Two raw queries in `server/plugins/dockerdisc/script.py`:
- `resolve_host_mac()`: `SELECT devMac FROM Devices WHERE devName = ? COLLATE NOCASE` — a name lookup with real 0/1/many-match handling (falls back to a manually-configured MAC on ambiguity or no match). No existing method covers this. `devName` has no column-level collation, so the explicit `COLLATE NOCASE` here is correct, not redundant. **Fix: add `DeviceInstance.getAllByName(name)` returning every match** (not just one — the plugin's own ambiguity detection needs the full set), and have the plugin call that instead.
This is the shape of the fix in general: an existence/single-row check usually already has a model method; a query with plugin-specific result handling (ambiguity, filtering) usually needs a small new method added rather than a workaround in the plugin itself.
## The second check this skill adds: suspicious/attacker-influenced plugin data must be logged
A plugin that parses data from an unauthenticated peer (DHCP options, mDNS/Avahi records, NetBIOS name-service responses, SSDP/UPnP, any broadcast/discovery protocol) is trusting the network, not the device it's nominally scanning — any device on the segment can answer. When such a value gets rejected, sanitized, or otherwise flagged as suspicious/malformed, that's a security-relevant event, not routine parsing noise: **is it logged?**
At minimum, every such detection needs `mylog("none", ...)` (`logger.py`'s `debugLevels` — `"none"` is level 0, the always-shown floor, not filtered out at any configured `LOG_LEVEL` — matching how this codebase already logs real errors, e.g. `mylog("none", f"[Plugins] ⚠ ERROR: {e}")`). A silently-dropped or silently-mangled value with no log trace is the finding to raise — an admin investigating "why does this device's name look wrong" or "was my network probed" has nothing to go on otherwise.
**A user-facing alert (`write_notification()`, `server/messaging/in_app.py`) is a separate, materially bigger decision: don't require it as a blocking condition the way the log line is.** It's persistent and unprompted, and (per existing precedent: `api_server_start.py`'s unauthorized-access-attempt alert fires unconditionally, with no rate-limiting anywhere in this codebase) a repeat offender re-sending the same payload every scan cycle can spam it indefinitely unless the PR gates it correctly on `process_plugin_events()`'s existing per-object status (`server/plugin.py:769-791`): fire only on `"new"` or `"watched-changed"`, never on `"watched-not-changed"` (that status is set every cycle a value stays the same, so alerting on it defeats the suppression entirely). For a missing-object alert, fire only on the transition into `"missing-in-last-scan"` (`server/plugin.py`'s `if tmpObj.status != "missing-in-last-scan":` guard around line 807), not on every cycle the object remains in that status. If a PR adds `write_notification()` for this without matching that gating, that's the thing to flag, not the absence of a user-facing alert on its own.
**Worked example (generalized):** a plugin parses an unauthenticated broadcast-protocol response (e.g. a DHCP option, an mDNS/NetBIOS record) and copies a field from it verbatim into a stored value with no validation. The fix centralizes both the sanitization *and* the `mylog("none", ...)` call in one shared, plugin-agnostic enforcement point (`plugin_object_class.__init__`, `server/plugin.py`) rather than leaving individual plugin authors to remember either — the same reasoning as the raw-SQL check above: a check that depends on every plugin author independently thinking to add it will eventually ship without it.
## The third check this skill adds: a new dependency has to reach every build target the plugin should run on
If a PR adds a system package (`apk add` in the root `Dockerfile`) or a Python dependency (`requirements.txt`), check whether it actually reached every place that needs it, not just the one file the diff touched:
- **`.devcontainer/Dockerfile`** is a committed, git-tracked file generated by concatenating the root `Dockerfile` with `.devcontainer/resources/devcontainer-Dockerfile` (`.devcontainer/scripts/generate-configs.sh`). It is not read fresh from the root `Dockerfile` at build time, so a PR that edits the root `Dockerfile` without re-running that script leaves this file stale, silently missing the new package for anyone testing inside the devcontainer. CI enforces this: the `check-devcontainer-dockerfile` job (`.github/workflows/code-checks.yml`) regenerates the file and fails the build if it doesn't match the committed one.
- **`Dockerfile.debian`** (`docs/BUILDS.md`) is a second, separately-maintained build target with its own `apt-get install` list and `setcap` calls. A system package (and its capability grant, if the plugin needs one, like `arp-scan`/`nmap`/`iw`) added only to the Alpine `Dockerfile` leaves this target broken.
- **`install/ubuntu24/requirements.txt`** and **`install/proxmox/requirements.txt`** are separate Python dependency lists for their own non-Docker install methods, not mechanically synced with the root `requirements.txt` (they already drift from it today) - check whether the new dependency is actually needed by those install paths too.
For the latter two, `scripts/check_dependency_mirroring.py` (wired into the non-blocking `check-dependency-mirroring` CI job) flags a PR that touches `Dockerfile` without `Dockerfile.debian`, or `requirements.txt` without both `install/*` copies - it only knows the sibling file wasn't touched at all, not whether the specific package was actually needed there, so treat a flag as a prompt to check, not a verdict.
A worked example: a WiFi-scanning plugin PR added `iw` plus its `setcap` grant to the root `Dockerfile` only. The devcontainer's checked-in Dockerfile went stale (missing `iw` until someone regenerates it), and `Dockerfile.debian` never got `iw` or a `setcap` line for it at all - the plugin silently can't scan on either target, caught only because the plugin's own error handling logs "not found" rather than crashing.
+7 -6
View File
@@ -22,20 +22,21 @@ Both are plausible, well-written, and wrong. Reading the code first catches both
1. **Understand the current mechanism by reading the actual code before writing anything.** Cite `file:line` for every claim about current behavior. Delegate to an Explore/general-purpose agent for breadth if the surface area is large, but treat its findings as a starting point to spot-check, not a finished citation — verify anything load-bearing yourself before it goes in the PRD. The same applies to a prior audit or PRD this one continues from: re-read its full detail section for the specific finding, not just a one-line summary-table row, before citing or extending it — a summary row can omit a caveat ("already indexed," "already fixed elsewhere") that only the detail text states, and citing the row alone can reintroduce a claim the detail text already corrected.
2. **Challenge the idea before designing it.** If the user proposes a solution, ask: is this solving the right problem? Does it conflate unrelated concerns (see axis-separation, next)? Does a similar or previously-rejected mechanism already exist that this would collide with semantically? A naming near-collision with an existing field/concept that has different, incompatible semantics is a signal to stop and check precedence rules, not a coincidence to wave off.
3. **Identify the independent axes.** A feature request that arrives as "option A and option B" is often two or three orthogonal concerns bundled together — e.g. "should this exist at all," "should it notify," and "should it assert presence" are three separate questions, not one. Cramming them into a single enum/flag produces combinations you can't express later (what if a plugin wants A+C but not B?). Give each axis its own mechanism.
3. **Identify the independent axes.** A feature request that arrives as "option A and option B" is often two or three orthogonal concerns bundled together — e.g. "should this exist at all," "should it notify," and "should it assert presence" are three separate questions, not one. Cramming them into a single enum/flag produces combinations you can't express later (what if a plugin wants A+C but not B?). Give each axis its own mechanism. A recurring instance of this: if the design detects or handles data that indicates a likely security-relevant event (a plugin sanitizing suspicious/attacker-influenced input, a rejected/malformed value, an auth failure, etc.), "log it" and "alert the user about it" are two separate axes, not one. Server-log visibility (`mylog("none", ...)` in this codebase — the always-shown floor, matching how real errors are already logged) is close to a hard requirement whenever such an event is detected at all; a user-facing alert (`write_notification()`) is a materially bigger decision — it's persistent, unprompted, and (per existing precedent, e.g. `api_server_start.py`'s unauthorized-access alert) not rate-limited anywhere in this codebase, so a repeat offender can spam it. Don't let "should we detect/handle this" and "should we alert the user about it" collapse into one decision — record the logging as close to a given, and the user-facing alert as its own, explicit, separately-decidable (and separately deferrable) open issue.
4. **For every mechanism, trace every downstream consumer — not just the first one you find.** The single highest-value question before calling a design complete: "where else does this exact same check or logic get independently re-derived?" In a codebase without one source of truth for a concept (e.g. "is this record currently active" computed by three different queries in three different files), patching the first occurrence and stopping is the most common way a design ships with a hidden, silent gap. Grep for the pattern, not just the function you already know about.
5. **Record rejected alternatives with the reasoning, not just the chosen design.** Give it its own subsection (`### Rejected: X`). Without this, a future reader — or your own future self — re-proposes the rejected idea because the "why not" only ever existed in a conversation, not in the document.
6. **Force every open question to an explicit decision**, even if the decision is "accept as-is for v1, revisit if feedback says otherwise." An open question left unresolved in a PRD gets silently decided by whoever implements it — usually differently than anyone actually intended.
6. **Force every open question to an explicit decision**, even if the decision is "accept as-is for v1, revisit if feedback says otherwise." An open question left unresolved in a PRD gets silently decided by whoever implements it — usually differently than anyone intended.
7. **Write the test plan as part of the PRD, not after.** Concrete test cases — naming real functions/queries, not "add tests for X" — force you to notice design gaps you'd otherwise miss; the moment you try to write "assert Y happens" and realize the current design can't produce Y is often the first time the gap becomes visible. Check the repo for an existing test pattern for this shape of change before inventing a new one (e.g. a prior presence-logic bug fixed via `test/db_test_helpers.py` fixtures is the template for the next one, not a reason to build new test infrastructure).
8. **Ask explicitly whether validating this needs real end-to-end infrastructure** (a new or modified plugin, a UI click-through) or whether synthetic unit-level fixtures suffice — don't assume either way. Check whether the functions under test take a DB connection/dict/list as a parameter (testable in isolation, no real plugin needed) or require a real file on disk (harder to fake, may need one).
9. **Check performance against the real schema and real scale, not assumptions.** For every new or changed query: does it use an existing index, or add an unindexed lookup, a new join, or a correlated subquery? Grep `CREATE INDEX` for *every* index on the tables involved, not just the first one you find — a column can have both a plain index and a separate expression index (e.g. `idx_eve_mac_date_type ON Events(eveMac, ...)` alongside `idx_eve_lower_mac_date_type ON Events(LOWER(eveMac), ...)`), and missing the second one produces a wrong verdict. Then weigh cost by how often the query runs (once is nothing; every few minutes forever is a standing cost) and by real scale — **production users run 10,000+ devices**, not a homelab handful. A `CurrentScan` with 2-5 rows per device (one per contributing plugin) is routinely 20,000-50,000+ rows in one cycle; reason about that number, not a smaller hopeful one. A correlated `EXISTS`/subquery re-evaluated per outer row is fine *if* the correlated column is indexed — e.g. `current_scan_presence_condition()` (`server/scan/presence.py`) is exactly this shape against `CurrentScan.scanMac`, covered by `idx_currentscan_scanmac`. The real risk is an unindexed correlated lookup: an accidental self-join scanning the full inner table per outer row, which looks fine and passes tests at small scale but isn't at production scale. Check with `EXPLAIN QUERY PLAN` at a realistic row count, built against the *complete* real index set (copy every `CREATE INDEX` for the table, or run it against an actual `app.db`) rather than a hand-picked subset — a partial index set produces a misleading plan in either direction, not just "looks worse than it is." If it comes back unindexed for real, a `GROUP BY` aggregate is the usual fix.
10. **Do a dedicated final-check pass, out loud, before calling it done.** Re-read the whole document end to end and specifically check:
- Did a correction made mid-document actually propagate everywhere it needed to (the Design subsection *and* Affected Files *and* Tests *and* any execution-plan summary)? A correction landing in one place and not its siblings is worse than never catching it, because now the document silently contradicts itself.
- Did a correction made mid-document propagate everywhere it needed to (the Design subsection *and* Affected Files *and* Tests *and* any execution-plan summary)? A correction landing in one place and not its siblings is worse than never catching it, because now the document silently contradicts itself.
- Does every "this is the cleanest/simplest real case" claim still hold up if you actually re-read that specific piece of code right now, or was it asserted by pattern-matching a name/category? Re-verify, don't re-assert.
- Does anything render incorrectly as markdown — an unfenced ASCII diagram or code block will collapse into one line under lazy-paragraph-continuation, the same class of bug as a list missing its preceding blank line.
- Do any internal anchor links' slugs actually match their headings?
- Do any internal anchor links' slugs match their headings?
- Does the design still cleanly separate its axes, or did a later addition quietly re-conflate two concerns inside what's supposed to be a single-purpose mechanism (the same mistake step 3 exists to catch at the top level can reappear one level down inside an individual mechanism's own value set — e.g. a 3-value enum where two of the values are secretly independent booleans in a trenchcoat).
11. **Leave a visible trail of corrections instead of silently rewriting.** When a review pass — yours or someone else's — finds something wrong, write "**Correction (caught in review):** ..." inline rather than quietly fixing the earlier text and moving on. This is what makes a PRD trustworthy to a second reader: they can see what was checked and what changed, not just receive a polished final answer with no visible seams.
12. **When marking a PRD Implemented, ask once whether anything found along the way generalizes beyond this one document** — a tooling trick, a wrong assumption that took real digging to disprove, a review catch that would recur on the next PRD. The PRD's own "Implementation notes" section is the right place for what happened *in this PRD*; a skill or memory update is the right place for anything that would otherwise have to be rediscovered next time. One question, not a mandatory new section — skip it when nothing generalizes, don't force it on every PRD regardless of size.
## Structure to follow
@@ -45,7 +46,7 @@ Both are plausible, well-written, and wrong. Reading the code first catches both
- **Open issues** — each with an explicit recorded decision (step 6), not left dangling.
- **Affected files** — concrete `file:function:line` references, not bare filenames.
- **Backward compatibility** — explicit default values and why they preserve current behavior for existing consumers.
- **Performance impact** — the baseline (what's unindexed/slow *today*, independent of this change), what the change adds that's negligible, what's genuinely new and worth mitigating, and concrete mitigations rather than a vague "should be fine" (step 9).
- **Performance impact** — the baseline (what's unindexed/slow *today*, independent of this change), what the change adds that's negligible, what's new and worth mitigating, and concrete mitigations rather than a vague "should be fine" (step 9).
- **Docs/skills to update** — anywhere this needs to be reflected outside the code itself (external docs, paired skill files, template files new authors copy from).
- **Tests** — organized by mechanism, each case naming the real function/query it exercises and the concrete assertion (step 7), plus a manual verification checklist for anything that can't be unit-tested (including an `EXPLAIN QUERY PLAN` check at realistic scale if the Performance impact section found a genuine risk).
- **(Optional) Execution plan** — phased, referencing the same file/function names used above rather than restating the design in vaguer terms.
@@ -56,4 +57,4 @@ If a skill already documents the subsystem the feature touches, load it before r
## Where to save
`.gemini/internal-docs/PRDs/<kebab-case-name>.md`, unless the user specifies otherwise. Mark the status line (`**Status:** Draft — pending review`) so it's clear this hasn't been approved yet, and keep the author line accurate about who actually made the calls (a design discussion with an assistant is not sole assistant authorship).
`.gemini/internal-docs/PRDs/<kebab-case-name>.md`, unless the user specifies otherwise. Mark the status line (`**Status:** Draft — pending review`) so it's clear this hasn't been approved yet, and keep the author line accurate about who made the calls (a design discussion with an assistant is not sole assistant authorship).
+1 -1
View File
@@ -34,7 +34,7 @@ Covers what happens after a plugin's rows land in `CurrentScan`: presence comput
6. `update_presence_from_CurrentScan(db)` — sets `devPresentLastScan` from `CurrentScan` for this cycle (step 2 reads this as "previous" on the *next* cycle).
7. `update_devPresentLastScan_based_on_nics(db)` — NIC/parent-child presence aggregation; can override step 6 for parent devices.
8. `update_devPresentLastScan_based_on_force_status(db)` — the user's manual `devForceStatus` override; runs last, wins over everything above.
9. `update_vendors_from_mac`, `update_ipv4_ipv6`, `update_icons_and_types` — `update_ipv4_ipv6()` does **not** go through `LatestDeviceScan`/`FIELD_SPECS`; it reads `CurrentScan` directly with its own `PARTITION BY scanMac, address_family` ranking, so a device reporting both an IPv4 and an IPv6 row in the same cycle gets both `devPrimaryIPv4`/`devPrimaryIPv6` set from that cycle, not just whichever family happened to win `devLastIP`'s single-value reduction. See `.gemini/internal-docs/PRDs/dual-stack-primary-ip-support.md`.
9. `update_vendors_from_mac`, `update_ipv4_ipv6`, `update_icons_and_types` — `update_ipv4_ipv6()` does **not** go through `LatestDeviceScan`/`FIELD_SPECS`; it reads `CurrentScan` directly with its own `PARTITION BY scanMac, address_family` ranking, so a device reporting both an IPv4 and an IPv6 row in the same cycle gets both `devPrimaryIPv4`/`devPrimaryIPv6` set from that cycle, not just whichever family happened to win `devLastIP`'s single-value reduction.
10. `pair_sessions_events(db)` — pairs `Events` rows as described above.
11. `create_sessions_snapshot(db)` — `DELETE FROM Sessions; INSERT INTO Sessions SELECT * FROM Convert_Events_to_Sessions`. `Sessions` reflects step 10's pairing from here.
12. `insertOnlineHistory(db)` — dashboard graph rollup.
+7 -2
View File
@@ -27,7 +27,11 @@ Cut anything that narrates the past instead of stating the present:
## Rule 2: plain words, fewer words
If a shorter or simpler phrasing says the same thing, use it. Cut qualifiers that don't change the meaning ("actually," "really," "genuinely," "in this exact process"). Prefer a plain verb over a nominalization. A dense skill with real information beats a padded one — trim narration and hedging before trimming facts.
If a shorter or simpler phrasing says the same thing, use it. Cut qualifiers that don't change the meaning ("actually," "really," "genuinely," "in this exact process"). Prefer a plain verb over a nominalization. A dense skill with real information beats a padded one: trim narration and hedging before trimming facts.
## Rule 3: no em-dashes
Never use an em-dash ("—"), in a skill or anywhere else this session writes prose (docs, code comments, PRDs, commit messages, chat replies). Use a period, comma, colon, semicolon, or parentheses instead, whichever actually fits the sentence.
## Sweep before calling a skill clean
@@ -35,9 +39,10 @@ Run this across `.claude/skills/`, `.gemini/skills/`, `.github/skills/` (or a si
```bash
grep -rniE "as of 202|caught in review|caught mid-review|correction:|correction \(|shipped for real|previously|used to be|no longer|originally|was later|historically|in the past|distilled from|real mistake|it turned out|turns out|discovered that|during a past" .claude/skills/ .gemini/skills/ .github/skills/
grep -rn "—" .claude/skills/ .gemini/skills/ .github/skills/
```
Read every hit in context — some are legitimate (a rule instructing PRD authors to write correction trails, or "previously down" describing device state, are not violations). Fix the ones that narrate the skill's own history instead of the system's current behavior.
Read every hit in context: some are legitimate (a rule instructing PRD authors to write correction trails, or "previously down" describing device state, are not violations). Fix the ones that narrate the skill's own history instead of the system's current behavior, and replace every em-dash hit per Rule 3.
## Also applies to: research/audit docs
+25 -3
View File
@@ -1,6 +1,6 @@
---
name: netalertx-plugin-review
description: Read when reviewing a plugin PR or auditing an existing plugin script (server/plugins/*/script.py or equivalent). Covers the one check not already mechanically enforced by test_plugin_conventions.py - plugin scripts embedding their own raw SQL instead of an existing/new model method - plus a worked real-PR example.
description: Read when reviewing a plugin PR or auditing an existing plugin script (server/plugins/*/script.py or equivalent). Covers three checks not already mechanically enforced by test_plugin_conventions.py - plugin scripts embedding their own raw SQL instead of an existing/new model method, suspicious/attacker-influenced plugin data not being logged, and a new dependency not reaching every build target - plus worked real-PR examples.
---
# Plugin Review
@@ -20,8 +20,8 @@ Plugin scripts write their results to `RESULT_FILE` via `plugin_helper.Plugin_Ob
## Review flow for a raw SQL query in a plugin
1. **Does an existing model method already do this?** Check the relevant `server/models/*_instance.py` file (`DeviceInstance`, `EventInstance`, `PluginObjectInstance`, etc.) before assuming one needs to be added.
2. **If not, is it worth adding one** (`server/models/device_instance.py` etc.), or is this genuinely a one-off maintenance/schema query that belongs in the core-plugin exception list above?
3. **Check the collation the query relies on** against the column's actual schema (`server/db/schema/app.sql`) rather than assuming — `devMac`/`eveMac`/`sesMac`/`scanMac`/`devParentMAC` are declared `COLLATE NOCASE` at the column level, so an explicit `COLLATE NOCASE` against one of them in a new query is redundant (harmless, but a sign the author didn't check). `devName` has **no** column-level collation — an explicit `COLLATE NOCASE` there is genuinely necessary if case-insensitive name matching is intended, not a mistake.
2. **If not, is it worth adding one** (`server/models/device_instance.py` etc.), or is this a one-off maintenance/schema query that belongs in the core-plugin exception list above?
3. **Check the collation the query relies on** against the column's actual schema (`server/db/schema/app.sql`) rather than assuming — `devMac`/`eveMac`/`sesMac`/`scanMac`/`devParentMAC` are declared `COLLATE NOCASE` at the column level, so an explicit `COLLATE NOCASE` against one of them in a new query is redundant (harmless, but a sign the author didn't check). `devName` has **no** column-level collation — an explicit `COLLATE NOCASE` there is necessary if case-insensitive name matching is intended, not a mistake.
4. **Parameterization** — `?` placeholders, never string-formatted values into the query (this part is usually already fine; flag it if not).
## Worked example: PR #1788 (DOCKERDISC plugin)
@@ -32,3 +32,25 @@ Two raw queries in `server/plugins/dockerdisc/script.py`:
- `resolve_host_mac()`: `SELECT devMac FROM Devices WHERE devName = ? COLLATE NOCASE` — a name lookup with real 0/1/many-match handling (falls back to a manually-configured MAC on ambiguity or no match). No existing method covers this. `devName` has no column-level collation, so the explicit `COLLATE NOCASE` here is correct, not redundant. **Fix: add `DeviceInstance.getAllByName(name)` returning every match** (not just one — the plugin's own ambiguity detection needs the full set), and have the plugin call that instead.
This is the shape of the fix in general: an existence/single-row check usually already has a model method; a query with plugin-specific result handling (ambiguity, filtering) usually needs a small new method added rather than a workaround in the plugin itself.
## The second check this skill adds: suspicious/attacker-influenced plugin data must be logged
A plugin that parses data from an unauthenticated peer (DHCP options, mDNS/Avahi records, NetBIOS name-service responses, SSDP/UPnP, any broadcast/discovery protocol) is trusting the network, not the device it's nominally scanning — any device on the segment can answer. When such a value gets rejected, sanitized, or otherwise flagged as suspicious/malformed, that's a security-relevant event, not routine parsing noise: **is it logged?**
At minimum, every such detection needs `mylog("none", ...)` (`logger.py`'s `debugLevels` — `"none"` is level 0, the always-shown floor, not filtered out at any configured `LOG_LEVEL` — matching how this codebase already logs real errors, e.g. `mylog("none", f"[Plugins] ⚠ ERROR: {e}")`). A silently-dropped or silently-mangled value with no log trace is the finding to raise — an admin investigating "why does this device's name look wrong" or "was my network probed" has nothing to go on otherwise.
**A user-facing alert (`write_notification()`, `server/messaging/in_app.py`) is a separate, materially bigger decision: don't require it as a blocking condition the way the log line is.** It's persistent and unprompted, and (per existing precedent: `api_server_start.py`'s unauthorized-access-attempt alert fires unconditionally, with no rate-limiting anywhere in this codebase) a repeat offender re-sending the same payload every scan cycle can spam it indefinitely unless the PR gates it correctly on `process_plugin_events()`'s existing per-object status (`server/plugin.py:769-791`): fire only on `"new"` or `"watched-changed"`, never on `"watched-not-changed"` (that status is set every cycle a value stays the same, so alerting on it defeats the suppression entirely). For a missing-object alert, fire only on the transition into `"missing-in-last-scan"` (`server/plugin.py`'s `if tmpObj.status != "missing-in-last-scan":` guard around line 807), not on every cycle the object remains in that status. If a PR adds `write_notification()` for this without matching that gating, that's the thing to flag, not the absence of a user-facing alert on its own.
**Worked example (generalized):** a plugin parses an unauthenticated broadcast-protocol response (e.g. a DHCP option, an mDNS/NetBIOS record) and copies a field from it verbatim into a stored value with no validation. The fix centralizes both the sanitization *and* the `mylog("none", ...)` call in one shared, plugin-agnostic enforcement point (`plugin_object_class.__init__`, `server/plugin.py`) rather than leaving individual plugin authors to remember either — the same reasoning as the raw-SQL check above: a check that depends on every plugin author independently thinking to add it will eventually ship without it.
## The third check this skill adds: a new dependency has to reach every build target the plugin should run on
If a PR adds a system package (`apk add` in the root `Dockerfile`) or a Python dependency (`requirements.txt`), check whether it actually reached every place that needs it, not just the one file the diff touched:
- **`.devcontainer/Dockerfile`** is a committed, git-tracked file generated by concatenating the root `Dockerfile` with `.devcontainer/resources/devcontainer-Dockerfile` (`.devcontainer/scripts/generate-configs.sh`). It is not read fresh from the root `Dockerfile` at build time, so a PR that edits the root `Dockerfile` without re-running that script leaves this file stale, silently missing the new package for anyone testing inside the devcontainer. CI enforces this: the `check-devcontainer-dockerfile` job (`.github/workflows/code-checks.yml`) regenerates the file and fails the build if it doesn't match the committed one.
- **`Dockerfile.debian`** (`docs/BUILDS.md`) is a second, separately-maintained build target with its own `apt-get install` list and `setcap` calls. A system package (and its capability grant, if the plugin needs one, like `arp-scan`/`nmap`/`iw`) added only to the Alpine `Dockerfile` leaves this target broken.
- **`install/ubuntu24/requirements.txt`** and **`install/proxmox/requirements.txt`** are separate Python dependency lists for their own non-Docker install methods, not mechanically synced with the root `requirements.txt` (they already drift from it today) - check whether the new dependency is actually needed by those install paths too.
For the latter two, `scripts/check_dependency_mirroring.py` (wired into the non-blocking `check-dependency-mirroring` CI job) flags a PR that touches `Dockerfile` without `Dockerfile.debian`, or `requirements.txt` without both `install/*` copies - it only knows the sibling file wasn't touched at all, not whether the specific package was actually needed there, so treat a flag as a prompt to check, not a verdict.
A worked example: a WiFi-scanning plugin PR added `iw` plus its `setcap` grant to the root `Dockerfile` only. The devcontainer's checked-in Dockerfile went stale (missing `iw` until someone regenerates it), and `Dockerfile.debian` never got `iw` or a `setcap` line for it at all - the plugin silently can't scan on either target, caught only because the plugin's own error handling logs "not found" rather than crashing.
@@ -63,6 +63,8 @@ plugin_objects.add_object(...) # During processing
plugin_objects.write_result_file() # Exactly once at end
```
Every mapped field (`objectPrimaryId`/`objectSecondaryId`/`watchedValue1-4`/`extra`/`helpVal1-4`) is HTML/control-char-stripped by default before it's persisted: plugin output is untrusted (network responses, device-reported names, etc.). `foreignKey` is always sanitized too, unconditionally. Only opt a column out (`"allow_raw_text": true`) if it's a display-only type (`textarea_readonly`); see `docs/PLUGINS_DEV.md#field-sanitization`.
## Execution Phases
- `once`: runs once at startup
@@ -84,7 +86,9 @@ plugin_objects.write_result_file() # Exactly once at end
## Before Opening a PR
Check the plugin against the [Conventions Checklist](../../../docs/PLUGINS_DEV.md#conventions-checklist) in `docs/PLUGINS_DEV.md` — `RUN` default, schedule precedent, `RUN_TIMEOUT` semantics, reusing core settings, description length, and the multi-instance settings pattern. Most plugin PR review comments trace back to one of these.
Check the plugin against the [Conventions Checklist](../../../docs/PLUGINS_DEV.md#conventions-checklist) in `docs/PLUGINS_DEV.md` — `RUN` default, schedule precedent, `RUN_TIMEOUT` semantics, reusing core settings, description length, the multi-instance settings pattern, and `allow_raw_text` restricted to display-only column types. Most plugin PR review comments trace back to one of these.
If the plugin needs a new system package or Python dependency, mirroring it into the root `Dockerfile`/`requirements.txt` alone is not enough: see the Conventions Checklist's build-target-mirroring bullet for `.devcontainer/Dockerfile` (regenerate via `.devcontainer/scripts/generate-configs.sh`, don't hand-edit it), `Dockerfile.debian`, and `install/ubuntu24`/`install/proxmox`'s own `requirements.txt` files.
## Starting Point
+7 -6
View File
@@ -22,20 +22,21 @@ Both are plausible, well-written, and wrong. Reading the code first catches both
1. **Understand the current mechanism by reading the actual code before writing anything.** Cite `file:line` for every claim about current behavior. Delegate to an Explore/general-purpose agent for breadth if the surface area is large, but treat its findings as a starting point to spot-check, not a finished citation — verify anything load-bearing yourself before it goes in the PRD. The same applies to a prior audit or PRD this one continues from: re-read its full detail section for the specific finding, not just a one-line summary-table row, before citing or extending it — a summary row can omit a caveat ("already indexed," "already fixed elsewhere") that only the detail text states, and citing the row alone can reintroduce a claim the detail text already corrected.
2. **Challenge the idea before designing it.** If the user proposes a solution, ask: is this solving the right problem? Does it conflate unrelated concerns (see axis-separation, next)? Does a similar or previously-rejected mechanism already exist that this would collide with semantically? A naming near-collision with an existing field/concept that has different, incompatible semantics is a signal to stop and check precedence rules, not a coincidence to wave off.
3. **Identify the independent axes.** A feature request that arrives as "option A and option B" is often two or three orthogonal concerns bundled together — e.g. "should this exist at all," "should it notify," and "should it assert presence" are three separate questions, not one. Cramming them into a single enum/flag produces combinations you can't express later (what if a plugin wants A+C but not B?). Give each axis its own mechanism.
3. **Identify the independent axes.** A feature request that arrives as "option A and option B" is often two or three orthogonal concerns bundled together — e.g. "should this exist at all," "should it notify," and "should it assert presence" are three separate questions, not one. Cramming them into a single enum/flag produces combinations you can't express later (what if a plugin wants A+C but not B?). Give each axis its own mechanism. A recurring instance of this: if the design detects or handles data that indicates a likely security-relevant event (a plugin sanitizing suspicious/attacker-influenced input, a rejected/malformed value, an auth failure, etc.), "log it" and "alert the user about it" are two separate axes, not one. Server-log visibility (`mylog("none", ...)` in this codebase — the always-shown floor, matching how real errors are already logged) is close to a hard requirement whenever such an event is detected at all; a user-facing alert (`write_notification()`) is a materially bigger decision — it's persistent, unprompted, and (per existing precedent, e.g. `api_server_start.py`'s unauthorized-access alert) not rate-limited anywhere in this codebase, so a repeat offender can spam it. Don't let "should we detect/handle this" and "should we alert the user about it" collapse into one decision — record the logging as close to a given, and the user-facing alert as its own, explicit, separately-decidable (and separately deferrable) open issue.
4. **For every mechanism, trace every downstream consumer — not just the first one you find.** The single highest-value question before calling a design complete: "where else does this exact same check or logic get independently re-derived?" In a codebase without one source of truth for a concept (e.g. "is this record currently active" computed by three different queries in three different files), patching the first occurrence and stopping is the most common way a design ships with a hidden, silent gap. Grep for the pattern, not just the function you already know about.
5. **Record rejected alternatives with the reasoning, not just the chosen design.** Give it its own subsection (`### Rejected: X`). Without this, a future reader — or your own future self — re-proposes the rejected idea because the "why not" only ever existed in a conversation, not in the document.
6. **Force every open question to an explicit decision**, even if the decision is "accept as-is for v1, revisit if feedback says otherwise." An open question left unresolved in a PRD gets silently decided by whoever implements it — usually differently than anyone actually intended.
6. **Force every open question to an explicit decision**, even if the decision is "accept as-is for v1, revisit if feedback says otherwise." An open question left unresolved in a PRD gets silently decided by whoever implements it — usually differently than anyone intended.
7. **Write the test plan as part of the PRD, not after.** Concrete test cases — naming real functions/queries, not "add tests for X" — force you to notice design gaps you'd otherwise miss; the moment you try to write "assert Y happens" and realize the current design can't produce Y is often the first time the gap becomes visible. Check the repo for an existing test pattern for this shape of change before inventing a new one (e.g. a prior presence-logic bug fixed via `test/db_test_helpers.py` fixtures is the template for the next one, not a reason to build new test infrastructure).
8. **Ask explicitly whether validating this needs real end-to-end infrastructure** (a new or modified plugin, a UI click-through) or whether synthetic unit-level fixtures suffice — don't assume either way. Check whether the functions under test take a DB connection/dict/list as a parameter (testable in isolation, no real plugin needed) or require a real file on disk (harder to fake, may need one).
9. **Check performance against the real schema and real scale, not assumptions.** For every new or changed query: does it use an existing index, or add an unindexed lookup, a new join, or a correlated subquery? Grep `CREATE INDEX` for *every* index on the tables involved, not just the first one you find — a column can have both a plain index and a separate expression index (e.g. `idx_eve_mac_date_type ON Events(eveMac, ...)` alongside `idx_eve_lower_mac_date_type ON Events(LOWER(eveMac), ...)`), and missing the second one produces a wrong verdict. Then weigh cost by how often the query runs (once is nothing; every few minutes forever is a standing cost) and by real scale — **production users run 10,000+ devices**, not a homelab handful. A `CurrentScan` with 2-5 rows per device (one per contributing plugin) is routinely 20,000-50,000+ rows in one cycle; reason about that number, not a smaller hopeful one. A correlated `EXISTS`/subquery re-evaluated per outer row is fine *if* the correlated column is indexed — e.g. `current_scan_presence_condition()` (`server/scan/presence.py`) is exactly this shape against `CurrentScan.scanMac`, covered by `idx_currentscan_scanmac`. The real risk is an unindexed correlated lookup: an accidental self-join scanning the full inner table per outer row, which looks fine and passes tests at small scale but isn't at production scale. Check with `EXPLAIN QUERY PLAN` at a realistic row count, built against the *complete* real index set (copy every `CREATE INDEX` for the table, or run it against an actual `app.db`) rather than a hand-picked subset — a partial index set produces a misleading plan in either direction, not just "looks worse than it is." If it comes back unindexed for real, a `GROUP BY` aggregate is the usual fix.
10. **Do a dedicated final-check pass, out loud, before calling it done.** Re-read the whole document end to end and specifically check:
- Did a correction made mid-document actually propagate everywhere it needed to (the Design subsection *and* Affected Files *and* Tests *and* any execution-plan summary)? A correction landing in one place and not its siblings is worse than never catching it, because now the document silently contradicts itself.
- Did a correction made mid-document propagate everywhere it needed to (the Design subsection *and* Affected Files *and* Tests *and* any execution-plan summary)? A correction landing in one place and not its siblings is worse than never catching it, because now the document silently contradicts itself.
- Does every "this is the cleanest/simplest real case" claim still hold up if you actually re-read that specific piece of code right now, or was it asserted by pattern-matching a name/category? Re-verify, don't re-assert.
- Does anything render incorrectly as markdown — an unfenced ASCII diagram or code block will collapse into one line under lazy-paragraph-continuation, the same class of bug as a list missing its preceding blank line.
- Do any internal anchor links' slugs actually match their headings?
- Do any internal anchor links' slugs match their headings?
- Does the design still cleanly separate its axes, or did a later addition quietly re-conflate two concerns inside what's supposed to be a single-purpose mechanism (the same mistake step 3 exists to catch at the top level can reappear one level down inside an individual mechanism's own value set — e.g. a 3-value enum where two of the values are secretly independent booleans in a trenchcoat).
11. **Leave a visible trail of corrections instead of silently rewriting.** When a review pass — yours or someone else's — finds something wrong, write "**Correction (caught in review):** ..." inline rather than quietly fixing the earlier text and moving on. This is what makes a PRD trustworthy to a second reader: they can see what was checked and what changed, not just receive a polished final answer with no visible seams.
12. **When marking a PRD Implemented, ask once whether anything found along the way generalizes beyond this one document** — a tooling trick, a wrong assumption that took real digging to disprove, a review catch that would recur on the next PRD. The PRD's own "Implementation notes" section is the right place for what happened *in this PRD*; a skill or memory update is the right place for anything that would otherwise have to be rediscovered next time. One question, not a mandatory new section — skip it when nothing generalizes, don't force it on every PRD regardless of size.
## Structure to follow
@@ -45,7 +46,7 @@ Both are plausible, well-written, and wrong. Reading the code first catches both
- **Open issues** — each with an explicit recorded decision (step 6), not left dangling.
- **Affected files** — concrete `file:function:line` references, not bare filenames.
- **Backward compatibility** — explicit default values and why they preserve current behavior for existing consumers.
- **Performance impact** — the baseline (what's unindexed/slow *today*, independent of this change), what the change adds that's negligible, what's genuinely new and worth mitigating, and concrete mitigations rather than a vague "should be fine" (step 9).
- **Performance impact** — the baseline (what's unindexed/slow *today*, independent of this change), what the change adds that's negligible, what's new and worth mitigating, and concrete mitigations rather than a vague "should be fine" (step 9).
- **Docs/skills to update** — anywhere this needs to be reflected outside the code itself (external docs, paired skill files, template files new authors copy from).
- **Tests** — organized by mechanism, each case naming the real function/query it exercises and the concrete assertion (step 7), plus a manual verification checklist for anything that can't be unit-tested (including an `EXPLAIN QUERY PLAN` check at realistic scale if the Performance impact section found a genuine risk).
- **(Optional) Execution plan** — phased, referencing the same file/function names used above rather than restating the design in vaguer terms.
@@ -56,4 +57,4 @@ If a skill already documents the subsystem the feature touches, load it before r
## Where to save
`.gemini/internal-docs/PRDs/<kebab-case-name>.md`, unless the user specifies otherwise. Mark the status line (`**Status:** Draft — pending review`) so it's clear this hasn't been approved yet, and keep the author line accurate about who actually made the calls (a design discussion with an assistant is not sole assistant authorship).
`.gemini/internal-docs/PRDs/<kebab-case-name>.md`, unless the user specifies otherwise. Mark the status line (`**Status:** Draft — pending review`) so it's clear this hasn't been approved yet, and keep the author line accurate about who made the calls (a design discussion with an assistant is not sole assistant authorship).
+1 -1
View File
@@ -34,7 +34,7 @@ Covers what happens after a plugin's rows land in `CurrentScan`: presence comput
6. `update_presence_from_CurrentScan(db)` — sets `devPresentLastScan` from `CurrentScan` for this cycle (step 2 reads this as "previous" on the *next* cycle).
7. `update_devPresentLastScan_based_on_nics(db)` — NIC/parent-child presence aggregation; can override step 6 for parent devices.
8. `update_devPresentLastScan_based_on_force_status(db)` — the user's manual `devForceStatus` override; runs last, wins over everything above.
9. `update_vendors_from_mac`, `update_ipv4_ipv6`, `update_icons_and_types` — `update_ipv4_ipv6()` does **not** go through `LatestDeviceScan`/`FIELD_SPECS`; it reads `CurrentScan` directly with its own `PARTITION BY scanMac, address_family` ranking, so a device reporting both an IPv4 and an IPv6 row in the same cycle gets both `devPrimaryIPv4`/`devPrimaryIPv6` set from that cycle, not just whichever family happened to win `devLastIP`'s single-value reduction. See `.gemini/internal-docs/PRDs/dual-stack-primary-ip-support.md`.
9. `update_vendors_from_mac`, `update_ipv4_ipv6`, `update_icons_and_types` — `update_ipv4_ipv6()` does **not** go through `LatestDeviceScan`/`FIELD_SPECS`; it reads `CurrentScan` directly with its own `PARTITION BY scanMac, address_family` ranking, so a device reporting both an IPv4 and an IPv6 row in the same cycle gets both `devPrimaryIPv4`/`devPrimaryIPv6` set from that cycle, not just whichever family happened to win `devLastIP`'s single-value reduction.
10. `pair_sessions_events(db)` — pairs `Events` rows as described above.
11. `create_sessions_snapshot(db)` — `DELETE FROM Sessions; INSERT INTO Sessions SELECT * FROM Convert_Events_to_Sessions`. `Sessions` reflects step 10's pairing from here.
12. `insertOnlineHistory(db)` — dashboard graph rollup.
+7 -2
View File
@@ -27,7 +27,11 @@ Cut anything that narrates the past instead of stating the present:
## Rule 2: plain words, fewer words
If a shorter or simpler phrasing says the same thing, use it. Cut qualifiers that don't change the meaning ("actually," "really," "genuinely," "in this exact process"). Prefer a plain verb over a nominalization. A dense skill with real information beats a padded one — trim narration and hedging before trimming facts.
If a shorter or simpler phrasing says the same thing, use it. Cut qualifiers that don't change the meaning ("actually," "really," "genuinely," "in this exact process"). Prefer a plain verb over a nominalization. A dense skill with real information beats a padded one: trim narration and hedging before trimming facts.
## Rule 3: no em-dashes
Never use an em-dash ("—"), in a skill or anywhere else this session writes prose (docs, code comments, PRDs, commit messages, chat replies). Use a period, comma, colon, semicolon, or parentheses instead, whichever actually fits the sentence.
## Sweep before calling a skill clean
@@ -35,9 +39,10 @@ Run this across `.claude/skills/`, `.gemini/skills/`, `.github/skills/` (or a si
```bash
grep -rniE "as of 202|caught in review|caught mid-review|correction:|correction \(|shipped for real|previously|used to be|no longer|originally|was later|historically|in the past|distilled from|real mistake|it turned out|turns out|discovered that|during a past" .claude/skills/ .gemini/skills/ .github/skills/
grep -rn "—" .claude/skills/ .gemini/skills/ .github/skills/
```
Read every hit in context — some are legitimate (a rule instructing PRD authors to write correction trails, or "previously down" describing device state, are not violations). Fix the ones that narrate the skill's own history instead of the system's current behavior.
Read every hit in context: some are legitimate (a rule instructing PRD authors to write correction trails, or "previously down" describing device state, are not violations). Fix the ones that narrate the skill's own history instead of the system's current behavior, and replace every em-dash hit per Rule 3.
## Also applies to: research/audit docs
+35
View File
@@ -127,3 +127,38 @@ jobs:
- name: 🔍 Check for skill-pair drift
continue-on-error: true
run: python3 scripts/check_skill_pairs.py "origin/${{ github.base_ref }}"
check-devcontainer-dockerfile:
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@v4
- name: 🔍 Check .devcontainer/Dockerfile is up to date
run: |
echo "Regenerating .devcontainer/Dockerfile from the root Dockerfile + .devcontainer/resources/devcontainer-Dockerfile..."
cp .devcontainer/Dockerfile /tmp/devcontainer-dockerfile-committed
bash .devcontainer/scripts/generate-configs.sh
if ! diff -q /tmp/devcontainer-dockerfile-committed .devcontainer/Dockerfile > /dev/null; then
echo "❌ .devcontainer/Dockerfile is stale."
echo "It's generated from the root Dockerfile and .devcontainer/resources/devcontainer-Dockerfile,"
echo "not hand-edited. Run 'bash .devcontainer/scripts/generate-configs.sh' locally and commit the result."
echo
diff /tmp/devcontainer-dockerfile-committed .devcontainer/Dockerfile || true
exit 1
fi
echo "✅ .devcontainer/Dockerfile is up to date."
check-dependency-mirroring:
if: github.event_name == 'pull_request'
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@v4
with:
fetch-depth: 0
- name: 🔍 Check for dependency-mirroring gaps
continue-on-error: true
run: python3 scripts/check_dependency_mirroring.py "origin/${{ github.base_ref }}"
+21
View File
@@ -245,6 +245,8 @@ Check your plugin against these repo-wide conventions before opening a PR (verif
- **Persist plugin state under `dbFolderPath`, config artifacts under `configPath`** — see [Persisting Plugin Data](#persisting-plugin-data-state--config-files) below.
- **A plugin mapped to `mapped_to_table: "CurrentScan"` must also map `scanSourcePlugin`** (a static value via `mapped_to_column_data`, see [Static Value Mapping](#static-value-mapping) below) — not mechanically enforced by `test_plugin_conventions.py`, so review it by eye. Omitting it leaves `scanSourcePlugin` `NULL` on every row this plugin inserts, which silently breaks two things in `server/scan/device_handling.py`: `create_new_devices()`'s `plugin_prefix = str(scanSourcePlugin).strip() if scanSourcePlugin else "NEWDEV"` mislabels devices this plugin creates as source `NEWDEV`; and `update_devices_data_from_scan()`'s `SELECT DISTINCT scanSourcePlugin FROM CurrentScan` + `[row[0] for row in plugin_rows if row[0]] or [None]` drops the `NULL` rows entirely (the `or [None]` fallback never triggers once any other plugin contributes a non-null prefix), so this plugin's `CurrentScan` rows never get picked up by the per-plugin device-update loop at all — the plugin can *insert* into `CurrentScan` but never actually confirm/update a device's presence.
- **A setting's `dataType` and `default_value` must actually agree.** `dataType: "array"` (or `"object"`) means `default_value` must be a real JSON literal for that shape — `'["default"]'`, not the bare string `"default"`. `setting_value_to_python_type()` (`server/helper.py`) `json.loads()`s the default at runtime; a bare string fails that parse, silently logs a decode error, and returns `[]` instead of your intended default — this shipped for real in `devParentRelType`/`UI_theme`/`UI_TOPOLOGY_ORDER` before being caught. If `elementOptions` already sets `multiple`/`orderable: "false"`, that's a strong signal the setting is actually scalar and `dataType` should be `"string"`, not `"array"`, regardless of what UI widget (`select`, etc.) renders it.
- **Only set `"allow_raw_text": true` on a display-only column type (`textarea_readonly`).** Every plugin-sourced field is HTML/control-char-stripped by default before it reaches the DB; see [Field Sanitization](#field-sanitization) below. `test_allow_raw_text_only_on_safe_types` (`test/plugins/test_plugin_conventions.py`) enforces the type restriction; it can't catch a column that legitimately needs the opt-out but is rendered somewhere unsafe, so use it only for values that are never interpreted as HTML.
- **A new system package or Python dependency needs mirroring across every build target it should work on, not just the root `Dockerfile`.** `.devcontainer/Dockerfile` is auto-generated (its own header says so) by `.devcontainer/scripts/generate-configs.sh`, which concatenates the root `Dockerfile` with `.devcontainer/resources/devcontainer-Dockerfile`. It's a committed, git-tracked file, not something read fresh at build time - a package added to the root `Dockerfile` without re-running that script leaves the devcontainer's checked-in Dockerfile stale, so the package is silently missing there until someone regenerates it. `Dockerfile.debian` (`docs/BUILDS.md`) is a second, separately-maintained build target with its own `apt-get install` package list and `setcap` calls - a system package (and its `setcap` grant, if it needs one) has to be added there too, not just to the Alpine `Dockerfile`. For a new Python dependency, the root `requirements.txt` is not the only one either: `install/ubuntu24/requirements.txt` and `install/proxmox/requirements.txt` are separate lists for their respective non-Docker install methods, not mechanically kept in sync with the root file (no CI check covers this) - check whether the new dependency is actually needed by those install methods too rather than assuming the root file alone is enough.
---
@@ -313,6 +315,25 @@ To always map a static value (not read from plugin output):
Every `mapped_to_table: "CurrentScan"` plugin needs this `scanSourcePlugin` mapping — see the Conventions Checklist above for what breaks downstream if it's left out.
### Field Sanitization
Plugin output is untrusted: it's parsed from network responses, device-reported names, headers, and similar attacker-influenceable sources. `plugin_object_class.__init__` (`server/plugin.py`) strips HTML tag-delimiter (`<`, `>`) and control characters from every mapped `objectPrimaryId`/`objectSecondaryId`/`watchedValue1-4`/`extra`/`helpVal1-4` field by default, via `plugin_helper.sanitize_plugin_text()`, before the value is persisted. `foreignKey` is sanitized unconditionally the same way, since it has no `database_column_definitions` entry of its own to attach an opt-out to.
This is defense-in-depth, not a substitute for output encoding: every renderer must still escape on display. It's also not a validator: a MAC-shaped `objectPrimaryId` still goes through `normalize_mac()` separately, and a malformed value is stripped of dangerous characters, not rejected or blanked.
A column only needs to opt out (`"allow_raw_text": true`) if it legitimately displays raw text that sanitization would otherwise mangle, e.g. a publisher plugin's raw API response body, shown in a `textarea_readonly` field:
```json
{
"column": "watchedValue2",
"type": "textarea_readonly",
"allow_raw_text": true,
"name": [{"language_code": "en_us", "string": "Response"}]
}
```
Restrict this to types that are never rendered as HTML; see the Conventions Checklist above.
### Import Behavior Columns (`scanCreatesDevice`, `scanNotificationMode`, `scanPresence`)
Three optional columns on `CurrentScan` control what happens once a row reaches it — see [Plugin Import Behavior](PLUGINS_IMPORT_BEHAVIOR.md) for the full contract (allowed values, defaults, downstream effects). All three default to today's behavior if never mapped, so existing plugins need no changes.
+7
View File
@@ -1,3 +1,4 @@
cryptography<40
openwrt-luci-rpc
asusrouter
aiohttp
@@ -23,6 +24,12 @@ dnspython
librouteros
yattag
zeroconf
simplejson
future
six
urllib3
httplib2
psutil
freebox-api
pydantic>=2.0,<3.0
fritzconnection>=1.15.1
+7
View File
@@ -1,3 +1,4 @@
cryptography<40
openwrt-luci-rpc
asusrouter
aiohttp
@@ -23,6 +24,12 @@ dnspython
librouteros
yattag
zeroconf
simplejson
future
six
urllib3
httplib2
psutil
freebox-api
pydantic>=2.0,<3.0
fritzconnection>=1.15.1
+66
View File
@@ -0,0 +1,66 @@
#!/usr/bin/env python3
"""
Flag PRs that add a system package (root Dockerfile) or a Python dependency
(root requirements.txt) without touching the sibling build targets that also
need it. See docs/PLUGINS_DEV.md#conventions-checklist and the
plugin-review skill's "a new dependency has to reach every build target"
check - a package added only to the Alpine Dockerfile leaves Dockerfile.debian
(a separately-maintained apt-based target, docs/BUILDS.md) without it, and a
Python package added only to the root requirements.txt may still be needed by
install/ubuntu24 or install/proxmox's own non-Docker install methods.
This can't know whether a given package is actually needed on the other
target (a real difference is legitimate and common), only that the PR didn't
touch the sibling file at all - a signal worth a human glance, not a
mechanical verdict. Exit non-zero (the CI step calling this is non-blocking).
python3 scripts/check_dependency_mirroring.py origin/main
"""
import subprocess
import sys
GROUPS = [
["Dockerfile", "Dockerfile.debian"],
["requirements.txt", "install/ubuntu24/requirements.txt", "install/proxmox/requirements.txt"],
]
def changed_files(base_ref):
result = subprocess.run(
["git", "diff", "--name-only", f"{base_ref}...HEAD"],
capture_output=True, text=True, check=True,
)
return set(result.stdout.splitlines())
def main():
if len(sys.argv) != 2:
print("usage: check_dependency_mirroring.py <base-ref>", file=sys.stderr)
return 2
changed = changed_files(sys.argv[1])
problems = []
for group in GROUPS:
touched = [path for path in group if path in changed]
untouched = [path for path in group if path not in changed]
if touched and untouched:
problems.append(
f"- touched {', '.join(touched)} but not {', '.join(untouched)}."
)
if problems:
print("Possible dependency-mirroring gap (a new package may need the other file(s) too):")
print("\n".join(problems))
print("\nIf the change is genuinely target-specific (e.g. a Debian-only fix, or a "
"dependency only the Docker build needs), ignore this. Otherwise check whether "
"the other file(s) need the same package - see "
"docs/PLUGINS_DEV.md#conventions-checklist.")
return 1
print("No dependency-mirroring gap detected.")
return 0
if __name__ == "__main__":
sys.exit(main())
+85 -4
View File
@@ -29,7 +29,7 @@ from models.notification_instance import NotificationInstance
from messaging.in_app import write_notification
from models.user_events_queue_instance import UserEventsQueueInstance
from utils.crypto_utils import generate_deterministic_guid
from plugin_helper import normalize_mac
from plugin_helper import normalize_mac, sanitize_plugin_text
# -------------------------------------------------------------------------------
@@ -766,6 +766,29 @@ def process_plugin_events(db, plugin, plugEventsArr):
mylog("debug", f"[Plugins] Existing objects from Plugins_Objects: {len(pluginObjects)}")
mylog("debug", f"[Plugins] Logged events from the plugin run : {len(pluginEvents)}")
# Reject this run's whole batch if two of its own events share an
# idsHash - nothing in the merge loop below expects that, and
# merging both into the same object would silently conflate two
# distinct discovered identities. Only checked within this run's
# own events, not against pre-existing pluginObjects - matching
# an existing object there is the normal "exists" case, not a
# collision. Doesn't pick a winner and doesn't touch the merge
# loop itself; a run with no collision behaves exactly as before.
seen_by_hash = {}
for tmpObjFromEvent in pluginEvents:
prior = seen_by_hash.get(tmpObjFromEvent.idsHash)
if prior is not None:
mylog(
"none",
f"[Plugins] {pluginPref}: identity-hash collision between "
f"({prior.primaryId!r}, {prior.secondaryId!r}) and "
f"({tmpObjFromEvent.primaryId!r}, {tmpObjFromEvent.secondaryId!r}) "
f"in this run's events - rejecting all {len(pluginEvents)} events "
"without persisting any of them.",
)
return
seen_by_hash[tmpObjFromEvent.idsHash] = tmpObjFromEvent
# Loop thru all current events and update the status to "exists" if the event matches an existing object
index = 0
for tmpObjFromEvent in pluginEvents:
@@ -1105,6 +1128,24 @@ def process_plugin_events(db, plugin, plugEventsArr):
return
# Maps a database_column_definitions "column" name to the plugin_object_class
# attribute it feeds, for the sanitize-by-default pass below. Same vocabulary
# already used by the CurrentScan-mapping loop in process_plugin_events().
_SANITIZE_COLUMN_MAP = {
"objectPrimaryId": "primaryId",
"objectSecondaryId": "secondaryId",
"watchedValue1": "watched1",
"watchedValue2": "watched2",
"watchedValue3": "watched3",
"watchedValue4": "watched4",
"extra": "extra",
"helpVal1": "helpVal1",
"helpVal2": "helpVal2",
"helpVal3": "helpVal3",
"helpVal4": "helpVal4",
}
# -------------------------------------------------------------------------------
class plugin_object_class:
def __init__(self, plugin, objDbRow):
@@ -1129,6 +1170,34 @@ class plugin_object_class:
self.helpVal2 = objDbRow[16]
self.helpVal3 = objDbRow[17]
self.helpVal4 = objDbRow[18]
# Sanitize plugin-sourced text fields by default (defense-in-depth
# against a plugin persisting HTML-dangerous content) - skip a field
# only if its config.json entry declares "allow_raw_text": true.
for col in plugin.get("database_column_definitions", []):
attr = _SANITIZE_COLUMN_MAP.get(col.get("column"))
if attr is None or col.get("allow_raw_text"):
continue
raw_value = getattr(self, attr)
sanitized = sanitize_plugin_text(raw_value)
if sanitized != raw_value:
mylog(
"none",
f"[Plugins] {plugin['unique_prefix']}.{col['column']} sanitized (HTML/control chars stripped): {raw_value!r} -> {sanitized!r}",
)
setattr(self, attr, sanitized)
# foreignKey has no database_column_definitions entry of its own (no
# config flag to attach an opt-out to), so it's sanitized unconditionally.
if self.foreignKey:
sanitized = sanitize_plugin_text(self.foreignKey)
if sanitized != self.foreignKey:
mylog(
"none",
f"[Plugins] {plugin['unique_prefix']}.foreignKey sanitized (HTML/control chars stripped): {self.foreignKey!r} -> {sanitized!r}",
)
self.foreignKey = sanitized
self.objectGUID = generate_deterministic_guid(
self.pluginPref, self.primaryId, self.secondaryId
)
@@ -1147,8 +1216,12 @@ class plugin_object_class:
objDbRow,
)
self.idsHash = str(hash(str(self.primaryId) + str(self.secondaryId)))
# self.idsHash = str(self.primaryId) + str(self.secondaryId)
# Hashing the pair, not their concatenation - str(primaryId) + str(secondaryId)
# would make ("ab", "c") and ("a", "bc") produce the identical string "abc"
# and therefore the identical hash, wrongly treating two distinct identities
# as one (and, since seen_by_hash above compares idsHash too, wrongly
# rejecting a legitimate batch as a collision).
self.idsHash = str(hash((str(self.primaryId), str(self.secondaryId))))
self.watchedClmns = []
self.watchedIndxs = []
@@ -1162,6 +1235,12 @@ class plugin_object_class:
(8, "watchedValue3"),
(9, "watchedValue4"),
]
indexAttrMapping = {
6: "watched1",
7: "watched2",
8: "watched3",
9: "watched4",
}
if setObj is not None:
self.watchedClmns = setObj["value"]
@@ -1171,9 +1250,11 @@ class plugin_object_class:
if clmName == mapping[1]:
self.watchedIndxs.append(mapping[0])
# Use the sanitized watched1-4 attributes, not the raw objDbRow values,
# so two events exposing the same sanitized value hash identically.
tmp = ""
for indx in self.watchedIndxs:
tmp += str(objDbRow[indx])
tmp += str(getattr(self, indexAttrMapping[indx]))
self.watchedHash = str(hash(tmp))
+142 -37
View File
@@ -5,7 +5,11 @@
"enabled": true,
"data_source": "script",
"show_ui": true,
"localized": ["display_name", "description", "icon"],
"localized": [
"display_name",
"description",
"icon"
],
"display_name": [
{
"language_code": "en_us",
@@ -37,7 +41,9 @@
"type": "none",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -52,7 +58,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -71,7 +79,9 @@
"type": "url",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -86,7 +96,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -105,7 +117,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -120,7 +134,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -144,7 +160,9 @@
"param": "`<a href='/report.php?guid=${value}'>Link</a>`"
}
],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -159,13 +177,16 @@
"type": "textarea_readonly",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
"string": "Result"
}
]
],
"allow_raw_text": true
},
{
"column": "watchedValue3",
@@ -174,7 +195,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -193,7 +216,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -212,7 +237,9 @@
"type": "textbox_save",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -248,7 +275,9 @@
"replacement": "<div style='text-align:center'><i class='fa-solid fa-question'></i></div>"
}
],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -267,7 +296,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -283,16 +314,28 @@
"settings": [
{
"function": "RUN",
"events": ["test"],
"events": [
"test"
],
"type": {
"dataType": "string",
"elements": [
{ "elementType": "select", "elementOptions": [], "transformers": [] }
{
"elementType": "select",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "disabled",
"options": ["disabled", "on_notification"],
"localized": ["name", "description"],
"options": [
"disabled",
"on_notification"
],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -321,14 +364,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "readonly": "true" }],
"elementOptions": [
{
"readonly": "true"
}
],
"transformers": []
}
]
},
"default_value": "python3 /app/server/plugins/_publisher_apprise/apprise.py",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -357,14 +407,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "type": "number" }],
"elementOptions": [
{
"type": "number"
}
],
"transformers": []
}
]
},
"default_value": 10,
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -395,12 +452,19 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "input", "elementOptions": [], "transformers": [] }
{
"elementType": "input",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -427,12 +491,22 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "select", "elementOptions": [], "transformers": [] }
{
"elementType": "select",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "url",
"options": ["url", "tag"],
"localized": ["name", "description"],
"options": [
"url",
"tag"
],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -459,12 +533,19 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "input", "elementOptions": [], "transformers": [] }
{
"elementType": "input",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -491,12 +572,19 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "input", "elementOptions": [], "transformers": [] }
{
"elementType": "input",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -523,12 +611,22 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "select", "elementOptions": [], "transformers": [] }
{
"elementType": "select",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "html",
"options": ["html", "text"],
"localized": ["name", "description"],
"options": [
"html",
"text"
],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -557,14 +655,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "type": "number" }],
"elementOptions": [
{
"type": "number"
}
],
"transformers": []
}
]
},
"default_value": 1024,
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
+173 -44
View File
@@ -5,7 +5,11 @@
"enabled": true,
"data_source": "script",
"show_ui": true,
"localized": ["display_name", "description", "icon"],
"localized": [
"display_name",
"description",
"icon"
],
"display_name": [
{
"language_code": "en_us",
@@ -37,7 +41,9 @@
"type": "none",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -52,7 +58,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -71,7 +79,9 @@
"type": "url",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -86,7 +96,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -105,7 +117,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -120,7 +134,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -144,7 +160,9 @@
"param": "`<a href='/report.php?guid=${value}'>${value}</a>`"
}
],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -159,13 +177,16 @@
"type": "textarea_readonly",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
"string": "Result"
}
]
],
"allow_raw_text": true
},
{
"column": "watchedValue3",
@@ -174,7 +195,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -193,7 +216,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -212,7 +237,9 @@
"type": "textbox_save",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -248,7 +275,9 @@
"replacement": "<div style='text-align:center'><i class='fa-solid fa-question'></i></div>"
}
],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -267,7 +296,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -283,16 +314,28 @@
"settings": [
{
"function": "RUN",
"events": ["test"],
"events": [
"test"
],
"type": {
"dataType": "string",
"elements": [
{ "elementType": "select", "elementOptions": [], "transformers": [] }
{
"elementType": "select",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "disabled",
"options": ["disabled", "on_notification"],
"localized": ["name", "description"],
"options": [
"disabled",
"on_notification"
],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -321,14 +364,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "readonly": "true" }],
"elementOptions": [
{
"readonly": "true"
}
],
"transformers": []
}
]
},
"default_value": "python3 /app/server/plugins/_publisher_email/email_smtp.py",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -357,14 +407,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "type": "number" }],
"elementOptions": [
{
"type": "number"
}
],
"transformers": []
}
]
},
"default_value": 20,
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -395,12 +452,19 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "input", "elementOptions": [], "transformers": [] }
{
"elementType": "input",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -429,14 +493,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "type": "number" }],
"elementOptions": [
{
"type": "number"
}
],
"transformers": []
}
]
},
"default_value": 587,
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -465,14 +536,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "type": "checkbox" }],
"elementOptions": [
{
"type": "checkbox"
}
],
"transformers": []
}
]
},
"default_value": false,
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -499,12 +577,19 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "input", "elementOptions": [], "transformers": [] }
{
"elementType": "input",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -533,14 +618,23 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "type": "password" }],
"transformers": ["prefix|base64"]
"elementOptions": [
{
"type": "password"
}
],
"transformers": [
"prefix|base64"
]
}
]
},
"default_value": "",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -569,14 +663,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "type": "checkbox" }],
"elementOptions": [
{
"type": "checkbox"
}
],
"transformers": []
}
]
},
"default_value": false,
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -605,14 +706,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "type": "checkbox" }],
"elementOptions": [
{
"type": "checkbox"
}
],
"transformers": []
}
]
},
"default_value": false,
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -639,12 +747,19 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "input", "elementOptions": [], "transformers": [] }
{
"elementType": "input",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "to@email.com",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -671,12 +786,19 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "input", "elementOptions": [], "transformers": [] }
{
"elementType": "input",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "NetAlertX <from@email.com>",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -695,12 +817,19 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "input", "elementOptions": [], "transformers": [] }
{
"elementType": "input",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "NetAlertX Report",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
+224 -60
View File
@@ -5,7 +5,11 @@
"enabled": true,
"data_source": "script",
"show_ui": true,
"localized": ["display_name", "description", "icon"],
"localized": [
"display_name",
"description",
"icon"
],
"display_name": [
{
"language_code": "en_us",
@@ -37,7 +41,9 @@
"type": "none",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -52,7 +58,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -71,7 +79,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -86,7 +96,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -106,7 +118,9 @@
"param": "`<a href='/report.php?guid=${value}'>${value}</a>`"
}
],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -121,13 +135,16 @@
"type": "textarea_readonly",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
"string": "Response"
}
]
],
"allow_raw_text": true
},
{
"column": "watchedValue3",
@@ -136,7 +153,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -151,7 +170,9 @@
"type": "device_mac",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -166,7 +187,9 @@
"type": "textbox_save",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -202,7 +225,9 @@
"replacement": "<div style='text-align:center'><i class='fa-solid fa-question'></i></div>"
}
],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -221,7 +246,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -237,16 +264,28 @@
"settings": [
{
"function": "RUN",
"events": ["test"],
"events": [
"test"
],
"type": {
"dataType": "string",
"elements": [
{ "elementType": "select", "elementOptions": [], "transformers": [] }
{
"elementType": "select",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "disabled",
"options": ["disabled", "on_notification"],
"localized": ["name", "description"],
"options": [
"disabled",
"on_notification"
],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -275,14 +314,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "readonly": "true" }],
"elementOptions": [
{
"readonly": "true"
}
],
"transformers": []
}
]
},
"default_value": "python3 /app/server/plugins/_publisher_ntfy/ntfy.py",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -311,14 +357,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "type": "number" }],
"elementOptions": [
{
"type": "number"
}
],
"transformers": []
}
]
},
"default_value": 10,
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -349,12 +402,19 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "input", "elementOptions": [], "transformers": [] }
{
"elementType": "input",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "https://ntfy.sh",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -381,12 +441,19 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "input", "elementOptions": [], "transformers": [] }
{
"elementType": "input",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -422,7 +489,10 @@
},
"default_value": "",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -441,12 +511,19 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "input", "elementOptions": [], "transformers": [] }
{
"elementType": "input",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -475,14 +552,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "type": "password" }],
"elementOptions": [
{
"type": "password"
}
],
"transformers": []
}
]
},
"default_value": "",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -509,12 +593,25 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "select", "elementOptions": [], "transformers": [] }
{
"elementType": "select",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "urgent",
"options": ["urgent", "high", "default", "low", "min"],
"localized": ["name", "description"],
"options": [
"urgent",
"high",
"default",
"low",
"min"
],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -535,14 +632,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "type": "checkbox" }],
"elementOptions": [
{
"type": "checkbox"
}
],
"transformers": []
}
]
},
"default_value": true,
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -561,12 +665,23 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "input", "elementOptions": [{ "type": "password" }], "transformers": [] }
{
"elementType": "input",
"elementOptions": [
{
"type": "password"
}
],
"transformers": []
}
]
},
"default_value": "",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -588,21 +703,41 @@
{
"elementType": "input",
"elementOptions": [
{ "placeholder": "Enter value" },
{ "suffix": "_in" },
{ "cssClasses": "col-sm-10" },
{ "prefillValue": "null" }
{
"placeholder": "Enter value"
},
{
"suffix": "_in"
},
{
"cssClasses": "col-sm-10"
},
{
"prefillValue": "null"
}
],
"transformers": []
},
{
"elementType": "button",
"elementOptions": [
{ "sourceSuffixes": ["_in"] },
{ "separator": "" },
{ "cssClasses": "col-xs-12" },
{ "onClick": "addList(this, false)" },
{ "getStringKey": "Gen_Add" }
{
"sourceSuffixes": [
"_in"
]
},
{
"separator": ""
},
{
"cssClasses": "col-xs-12"
},
{
"onClick": "addList(this, false)"
},
{
"getStringKey": "Gen_Add"
}
],
"transformers": []
},
@@ -610,31 +745,57 @@
"elementType": "select",
"elementHasInputValue": 1,
"elementOptions": [
{ "multiple": "true" },
{ "readonly": "true" },
{ "editable": "true" }
{
"multiple": "true"
},
{
"readonly": "true"
},
{
"editable": "true"
}
],
"transformers": []
},
{
"elementType": "button",
"elementOptions": [
{ "sourceSuffixes": [] },
{ "separator": "" },
{ "cssClasses": "col-xs-6" },
{ "onClick": "removeAllOptions(this)" },
{ "getStringKey": "Gen_Remove_All" }
{
"sourceSuffixes": []
},
{
"separator": ""
},
{
"cssClasses": "col-xs-6"
},
{
"onClick": "removeAllOptions(this)"
},
{
"getStringKey": "Gen_Remove_All"
}
],
"transformers": []
},
{
"elementType": "button",
"elementOptions": [
{ "sourceSuffixes": [] },
{ "separator": "" },
{ "cssClasses": "col-xs-6" },
{ "onClick": "removeFromList(this)" },
{ "getStringKey": "Gen_Remove_Last" }
{
"sourceSuffixes": []
},
{
"separator": ""
},
{
"cssClasses": "col-xs-6"
},
{
"onClick": "removeFromList(this)"
},
{
"getStringKey": "Gen_Remove_Last"
}
],
"transformers": []
}
@@ -642,7 +803,10 @@
},
"default_value": [],
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
+101 -27
View File
@@ -5,7 +5,11 @@
"enabled": true,
"data_source": "script",
"show_ui": true,
"localized": ["display_name", "description", "icon"],
"localized": [
"display_name",
"description",
"icon"
],
"display_name": [
{
"language_code": "en_us",
@@ -37,7 +41,9 @@
"type": "none",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -52,7 +58,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -71,7 +79,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -86,7 +96,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -106,7 +118,9 @@
"param": "`<a href='/report.php?guid=${value}'>${value}</a>`"
}
],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -121,13 +135,16 @@
"type": "textarea_readonly",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
"string": "Response"
}
]
],
"allow_raw_text": true
},
{
"column": "watchedValue3",
@@ -136,7 +153,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -151,7 +170,9 @@
"type": "device_mac",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -166,7 +187,9 @@
"type": "textbox_save",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -202,7 +225,9 @@
"replacement": "<div style='text-align:center'><i class='fa-solid fa-question'></i></div>"
}
],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -221,7 +246,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -237,16 +264,28 @@
"settings": [
{
"function": "RUN",
"events": ["test"],
"events": [
"test"
],
"type": {
"dataType": "string",
"elements": [
{ "elementType": "select", "elementOptions": [], "transformers": [] }
{
"elementType": "select",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "disabled",
"options": ["disabled", "on_notification"],
"localized": ["name", "description"],
"options": [
"disabled",
"on_notification"
],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -275,14 +314,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "readonly": "true" }],
"elementOptions": [
{
"readonly": "true"
}
],
"transformers": []
}
]
},
"default_value": "python3 /app/server/plugins/_publisher_pushover/pushover.py",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -311,14 +357,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "type": "number" }],
"elementOptions": [
{
"type": "number"
}
],
"transformers": []
}
]
},
"default_value": 10,
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -345,12 +398,19 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "input", "elementOptions": [], "transformers": [] }
{
"elementType": "input",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "USER_KEY",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -369,12 +429,19 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "input", "elementOptions": [], "transformers": [] }
{
"elementType": "input",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "APP_TOKEN",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -393,12 +460,19 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "input", "elementOptions": [], "transformers": [] }
{
"elementType": "input",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "DEVICE_NAME",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
+83 -23
View File
@@ -5,7 +5,11 @@
"enabled": true,
"data_source": "script",
"show_ui": true,
"localized": ["display_name", "description", "icon"],
"localized": [
"display_name",
"description",
"icon"
],
"display_name": [
{
"language_code": "en_us",
@@ -37,7 +41,9 @@
"type": "none",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -52,7 +58,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -71,7 +79,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -86,7 +96,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -106,7 +118,9 @@
"param": "`<a href='/report.php?guid=${value}'>${value}</a>`"
}
],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -121,13 +135,16 @@
"type": "textarea_readonly",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
"string": "Response"
}
]
],
"allow_raw_text": true
},
{
"column": "watchedValue3",
@@ -136,7 +153,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -151,7 +170,9 @@
"type": "device_mac",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -166,7 +187,9 @@
"type": "textbox_save",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -202,7 +225,9 @@
"replacement": "<div style='text-align:center'><i class='fa-solid fa-question'></i></div>"
}
],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -221,7 +246,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -237,16 +264,28 @@
"settings": [
{
"function": "RUN",
"events": ["test"],
"events": [
"test"
],
"type": {
"dataType": "string",
"elements": [
{ "elementType": "select", "elementOptions": [], "transformers": [] }
{
"elementType": "select",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "disabled",
"options": ["disabled", "on_notification"],
"localized": ["name", "description"],
"options": [
"disabled",
"on_notification"
],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -275,14 +314,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "readonly": "true" }],
"elementOptions": [
{
"readonly": "true"
}
],
"transformers": []
}
]
},
"default_value": "python3 /app/server/plugins/_publisher_pushsafer/pushsafer.py",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -311,14 +357,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "type": "number" }],
"elementOptions": [
{
"type": "number"
}
],
"transformers": []
}
]
},
"default_value": 10,
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -349,12 +402,19 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "input", "elementOptions": [], "transformers": [] }
{
"elementType": "input",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "ApiKey",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
+107 -29
View File
@@ -5,7 +5,11 @@
"enabled": true,
"data_source": "script",
"show_ui": true,
"localized": ["display_name", "description", "icon"],
"localized": [
"display_name",
"description",
"icon"
],
"display_name": [
{
"language_code": "en_us",
@@ -33,7 +37,9 @@
"type": "none",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -48,7 +54,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -67,7 +75,9 @@
"type": "url",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -82,7 +92,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -101,7 +113,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -116,7 +130,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -140,7 +156,9 @@
"param": "`<a href='/report.php?guid=${value}'>${value}</a>`"
}
],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -155,13 +173,16 @@
"type": "textarea_readonly",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
"string": "Result"
}
]
],
"allow_raw_text": true
},
{
"column": "watchedValue3",
@@ -170,7 +191,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -189,7 +212,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -208,7 +233,9 @@
"type": "textbox_save",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -244,7 +271,9 @@
"replacement": "<div style='text-align:center'><i class='fa-solid fa-question'></i></div>"
}
],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -263,7 +292,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -279,16 +310,28 @@
"settings": [
{
"function": "RUN",
"events": ["test"],
"events": [
"test"
],
"type": {
"dataType": "string",
"elements": [
{ "elementType": "select", "elementOptions": [], "transformers": [] }
{
"elementType": "select",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "disabled",
"options": ["disabled", "on_notification"],
"localized": ["name", "description"],
"options": [
"disabled",
"on_notification"
],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -313,14 +356,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "readonly": "true" }],
"elementOptions": [
{
"readonly": "true"
}
],
"transformers": []
}
]
},
"default_value": "python3 /app/server/plugins/_publisher_telegram/tg.py",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -349,14 +399,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "type": "number" }],
"elementOptions": [
{
"type": "number"
}
],
"transformers": []
}
]
},
"default_value": 10,
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -387,12 +444,19 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "input", "elementOptions": [], "transformers": [] }
{
"elementType": "input",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -411,12 +475,19 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "input", "elementOptions": [], "transformers": [] }
{
"elementType": "input",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -437,14 +508,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "type": "number" }],
"elementOptions": [
{
"type": "number"
}
],
"transformers": []
}
]
},
"default_value": 1024,
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
+131 -34
View File
@@ -5,7 +5,11 @@
"enabled": true,
"data_source": "script",
"show_ui": true,
"localized": ["display_name", "description", "icon"],
"localized": [
"display_name",
"description",
"icon"
],
"display_name": [
{
"language_code": "en_us",
@@ -37,7 +41,9 @@
"type": "none",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -52,7 +58,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -71,7 +79,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -86,7 +96,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -106,7 +118,9 @@
"param": "`<a href='/report.php?guid=${value}'>${value}</a>`"
}
],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -121,13 +135,16 @@
"type": "textarea_readonly",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
"string": "Response (stdout)"
}
]
],
"allow_raw_text": true
},
{
"column": "watchedValue3",
@@ -136,13 +153,16 @@
"type": "textarea_readonly",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
"string": "Response (stderr)"
}
]
],
"allow_raw_text": true
},
{
"column": "watchedValue4",
@@ -151,7 +171,9 @@
"type": "device_mac",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -166,7 +188,9 @@
"type": "textbox_save",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -202,7 +226,9 @@
"replacement": "<div style='text-align:center'><i class='fa-solid fa-question'></i></div>"
}
],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -221,7 +247,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -237,16 +265,28 @@
"settings": [
{
"function": "RUN",
"events": ["test"],
"events": [
"test"
],
"type": {
"dataType": "string",
"elements": [
{ "elementType": "select", "elementOptions": [], "transformers": [] }
{
"elementType": "select",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "disabled",
"options": ["disabled", "on_notification"],
"localized": ["name", "description"],
"options": [
"disabled",
"on_notification"
],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -275,14 +315,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "readonly": "true" }],
"elementOptions": [
{
"readonly": "true"
}
],
"transformers": []
}
]
},
"default_value": "python3 /app/server/plugins/_publisher_webhook/webhook.py",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -311,14 +358,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "type": "number" }],
"elementOptions": [
{
"type": "number"
}
],
"transformers": []
}
]
},
"default_value": 10,
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -349,12 +403,19 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "input", "elementOptions": [], "transformers": [] }
{
"elementType": "input",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -381,12 +442,23 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "select", "elementOptions": [], "transformers": [] }
{
"elementType": "select",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "json",
"options": ["json", "html", "text"],
"localized": ["name", "description"],
"options": [
"json",
"html",
"text"
],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -413,12 +485,23 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "select", "elementOptions": [], "transformers": [] }
{
"elementType": "select",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "GET",
"options": ["GET", "POST", "PUT"],
"localized": ["name", "description"],
"options": [
"GET",
"POST",
"PUT"
],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -447,14 +530,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "type": "number" }],
"elementOptions": [
{
"type": "number"
}
],
"transformers": []
}
]
},
"default_value": 1024,
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -481,12 +571,19 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "input", "elementOptions": [], "transformers": [] }
{
"elementType": "input",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
+86 -21
View File
@@ -2,7 +2,7 @@
"code_name": "icmp_scan",
"unique_prefix": "ICMP",
"plugin_type": "other",
"execution_order" : "Layer_4",
"execution_order": "Layer_4",
"enabled": true,
"data_source": "script",
"mapped_to_table": "CurrentScan",
@@ -16,7 +16,11 @@
"compare_use_quotes": true
}
],
"localized": ["display_name", "description", "icon"],
"localized": [
"display_name",
"description",
"icon"
],
"display_name": [
{
"language_code": "en_us",
@@ -46,11 +50,17 @@
"settings": [
{
"function": "RUN",
"events": ["run"],
"events": [
"run"
],
"type": {
"dataType": "string",
"elements": [
{ "elementType": "select", "elementOptions": [], "transformers": [] }
{
"elementType": "select",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "disabled",
@@ -61,7 +71,10 @@
"schedule",
"always_after_scan"
],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -77,11 +90,17 @@
},
{
"function": "MODE",
"events": ["run"],
"events": [
"run"
],
"type": {
"dataType": "string",
"elements": [
{ "elementType": "select", "elementOptions": [], "transformers": [] }
{
"elementType": "select",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "fping",
@@ -89,7 +108,10 @@
"fping",
"ping"
],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -110,14 +132,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "readonly": "true" }],
"elementOptions": [
{
"readonly": "true"
}
],
"transformers": []
}
]
},
"default_value": "python3 /app/server/plugins/icmp_scan/icmp.py",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -145,7 +174,10 @@
},
"default_value": "-i 0.5 -c 3",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -173,7 +205,10 @@
},
"default_value": ".*",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -255,7 +290,10 @@
},
"default_value": "*/5 * * * *",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -276,14 +314,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "type": "number" }],
"elementOptions": [
{
"type": "number"
}
],
"transformers": []
}
]
},
"default_value": 10,
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -304,17 +349,28 @@
"elements": [
{
"elementType": "select",
"elementOptions": [{ "multiple": "true", "orderable": "true" }],
"elementOptions": [
{
"multiple": "true",
"orderable": "true"
}
],
"transformers": []
}
]
},
"default_value": ["devMac", "devLastIP"],
"default_value": [
"devMac",
"devLastIP"
],
"options": [
"devMac",
"devLastIP"
],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -335,7 +391,12 @@
"elements": [
{
"elementType": "select",
"elementOptions": [{ "multiple": "true", "orderable": "true" }],
"elementOptions": [
{
"multiple": "true",
"orderable": "true"
}
],
"transformers": []
}
]
@@ -347,7 +408,10 @@
"devName",
"devSourcePlugin"
],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -448,7 +512,8 @@
"language_code": "en_us",
"string": "Output"
}
]
],
"allow_raw_text": true
},
{
"column": "watchedValue3",
+150 -38
View File
@@ -2,7 +2,7 @@
"code_name": "internet_ip",
"unique_prefix": "INTRNT",
"plugin_type": "device_scanner",
"execution_order" : "Layer_3",
"execution_order": "Layer_3",
"enabled": true,
"mapped_to_table": "CurrentScan",
"data_filters": [
@@ -16,7 +16,11 @@
],
"data_source": "script",
"show_ui": true,
"localized": ["display_name", "description", "icon"],
"localized": [
"display_name",
"description",
"icon"
],
"display_name": [
{
"language_code": "en_us",
@@ -63,16 +67,30 @@
"settings": [
{
"function": "RUN",
"events": ["run"],
"events": [
"run"
],
"type": {
"dataType": "string",
"elements": [
{ "elementType": "select", "elementOptions": [], "transformers": [] }
{
"elementType": "select",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "disabled",
"options": ["disabled", "once", "schedule", "always_after_scan"],
"localized": ["name", "description"],
"options": [
"disabled",
"once",
"schedule",
"always_after_scan"
],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -105,14 +123,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "readonly": "true" }],
"elementOptions": [
{
"readonly": "true"
}
],
"transformers": []
}
]
},
"default_value": "python3 /app/server/plugins/internet_ip/script.py prev_ip={prev_ip} INTRNT_DIG_GET_IP_ARG={INTRNT_DIG_GET_IP_ARG}",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -147,12 +172,19 @@
"type": {
"dataType": "string",
"elements": [
{ "elementType": "input", "elementOptions": [], "transformers": [] }
{
"elementType": "input",
"elementOptions": [],
"transformers": []
}
]
},
"default_value": "-4 myip.opendns.com @resolver1.opendns.com",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -215,7 +247,10 @@
},
"default_value": "*/5 * * * *",
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -252,14 +287,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "type": "number" }],
"elementOptions": [
{
"type": "number"
}
],
"transformers": []
}
]
},
"default_value": 30,
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -296,14 +338,21 @@
"elements": [
{
"elementType": "input",
"elementOptions": [{ "type": "number" }],
"elementOptions": [
{
"type": "number"
}
],
"transformers": []
}
]
},
"default_value": 3,
"options": [],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -324,19 +373,29 @@
"elements": [
{
"elementType": "select",
"elementOptions": [{ "multiple": "true", "orderable": "true"}],
"elementOptions": [
{
"multiple": "true",
"orderable": "true"
}
],
"transformers": []
}
]
},
"default_value": ["watchedValue1"],
"default_value": [
"watchedValue1"
],
"options": [
"watchedValue1",
"watchedValue2",
"watchedValue3",
"watchedValue4"
],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -369,19 +428,30 @@
"elements": [
{
"elementType": "select",
"elementOptions": [{ "multiple": "true", "orderable": "true"}],
"elementOptions": [
{
"multiple": "true",
"orderable": "true"
}
],
"transformers": []
}
]
},
"default_value": ["new", "watched-changed"],
"default_value": [
"new",
"watched-changed"
],
"options": [
"new",
"watched-changed",
"watched-not-changed",
"missing-in-last-scan"
],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -418,17 +488,28 @@
"elements": [
{
"elementType": "select",
"elementOptions": [{ "multiple": "true", "orderable": "true" }],
"elementOptions": [
{
"multiple": "true",
"orderable": "true"
}
],
"transformers": []
}
]
},
"default_value": ["devMac", "devLastIP"],
"default_value": [
"devMac",
"devLastIP"
],
"options": [
"devMac",
"devLastIP"
],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -449,7 +530,12 @@
"elements": [
{
"elementType": "select",
"elementOptions": [{ "multiple": "true", "orderable": "true" }],
"elementOptions": [
{
"multiple": "true",
"orderable": "true"
}
],
"transformers": []
}
]
@@ -461,7 +547,10 @@
"devType",
"devSourcePlugin"
],
"localized": ["name", "description"],
"localized": [
"name",
"description"
],
"name": [
{
"language_code": "en_us",
@@ -484,7 +573,9 @@
"type": "none",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -500,7 +591,9 @@
"type": "device_name_mac",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -524,7 +617,9 @@
"type": "device_ip",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -547,7 +642,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -566,13 +663,16 @@
"type": "textarea_readonly",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
"string": "Response"
}
]
],
"allow_raw_text": true
},
{
"column": "watchedValue3",
@@ -581,7 +681,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -597,7 +699,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -616,7 +720,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -639,7 +745,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -663,7 +771,9 @@
"type": "label",
"default_value": "",
"options": [],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
@@ -703,7 +813,9 @@
"replacement": "<div style='text-align:center'><i class='fa-solid fa-question'></i></div>"
}
],
"localized": ["name"],
"localized": [
"name"
],
"name": [
{
"language_code": "en_us",
+24
View File
@@ -264,6 +264,30 @@ def normalize_mac(mac):
return ':'.join(normalized_parts)
# -------------------------------------------------------------------
_UNSAFE_TEXT_RE = re.compile(r'[<>\x00-\x08\x0b\x0c\x0e-\x1f]') # tag delimiters + control chars (tab/LF/CR kept)
def sanitize_plugin_text(value):
"""
Strip HTML tag-delimiter and control characters from a plugin-sourced
value. Applied to every plugin field by default in plugin_object_class
(server/plugin.py) - skip only via a column's config.json
'allow_raw_text: true'. Defense-in-depth only - every renderer must
still escape on display, this does not replace that. Stripping (not
HTML-encoding) avoids double-encoding wherever the value is later
escaped for display. Deliberately does not truncate: length-limiting is
a data-integrity/DoS concern, not an XSS one, and belongs in a separate
mechanism if ever added.
:param value: the plugin-sourced value to sanitize, or None.
:return: the sanitized value, or None if value was None.
"""
if value is None:
return value
return _UNSAFE_TEXT_RE.sub('', str(value))
# -------------------------------------------------------------------
def per_item_timeout(run_timeout, item_count, floor=1):
"""
+24
View File
@@ -369,3 +369,27 @@ def test_run_timeout_not_reused_in_loop(plugin_name):
'plugin_helper.per_item_timeout() for a runtime-variable-length one. '
"See docs/PLUGINS_DEV.md#conventions-checklist:\n" + "\n".join(issues)
)
# Column types where the rendered value is never HTML-interpreted and a plugin
# legitimately needs to show raw text (e.g. an API response body) - the only
# types allowed to opt out of the default sanitize-on-persist pass via
# "allow_raw_text": true. See docs/PLUGINS_DEV.md#conventions-checklist.
_ALLOW_RAW_TEXT_TYPES = {"textarea_readonly"}
@pytest.mark.parametrize('plugin_name', _PLUGIN_NAMES)
def test_allow_raw_text_only_on_safe_types(plugin_name):
config = _load_config(plugin_name)
for col in config.get('database_column_definitions', []):
if not col.get('allow_raw_text'):
continue
col_type = col.get('type')
assert col_type in _ALLOW_RAW_TEXT_TYPES, (
f"{plugin_name}: column {col.get('column')!r} sets \"allow_raw_text\": true "
f"but has type {col_type!r}, not one of {sorted(_ALLOW_RAW_TEXT_TYPES)}. "
'allow_raw_text skips HTML/control-char stripping for this field - only safe '
'on a type that is never rendered as HTML (e.g. a read-only textarea), and '
'still requires the renderer to escape on display - see '
'docs/PLUGINS_DEV.md#conventions-checklist.'
)
+3 -5
View File
@@ -1,11 +1,9 @@
"""
Regression guard for server/app_state.py's updateState()/broadcast_state_update()
call - pluginsStates must reach the SSE broadcast payload, not just the
persisted app_state.json. See
.gemini/internal-docs/PRDs/to_review/execution-queue-fe-locking-fix.md's
post-implementation addendum: the original broadcast_state_update() call
never passed pluginsStates, so front/js/ui_components.js's watchPluginState()
(fix C1) waited on an SSE event that could never arrive.
persisted app_state.json. The original broadcast_state_update() call never
passed pluginsStates, so front/js/ui_components.js's watchPluginState() waited
on an SSE event that could never arrive.
"""
import os
@@ -0,0 +1,169 @@
"""
Tests for the identity-hash collision guard in process_plugin_events().
If two events in the same plugin run resolve to the same idsHash (same
sanitized primaryId + secondaryId), nothing in the merge loop expects that -
merging both into the same Plugins_Objects row would silently conflate two
distinct discovered identities. process_plugin_events() now rejects the
whole run's batch (persists nothing) when this happens, without picking a
winner, and leaves a run with no collision unaffected.
Run from inside the NetAlertX container - server/plugin.py isn't importable
standalone outside it (real conf/database/api imports).
pytest "test/server/test_plugin_identity_collision.py" -v
"""
import os
import sys
import pytest
# ---------------------------------------------------------------------------
# Path setup
# ---------------------------------------------------------------------------
INSTALL_PATH = os.getenv("NETALERTX_APP", "/app")
sys.path.extend([f"{INSTALL_PATH}/server/plugins", f"{INSTALL_PATH}/server"])
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
from db_test_helpers import ( # noqa: E402
make_plugin_db,
make_plugin_dict,
make_plugin_event_row,
seed_plugin_object,
plugin_objects_rows,
)
import plugin as plugin_module # noqa: E402
from plugin import process_plugin_events # noqa: E402
PREFIX = "TESTPLG"
@pytest.fixture
def plugin_db():
"""Yield a (PluginFakeDB, connection) backed by an in-memory SQLite database."""
db, conn = make_plugin_db()
yield db, conn
conn.close()
def _no_report_on(key, default=""):
"""Monkeypatch target: return empty REPORT_ON so no events are generated."""
if key.endswith("_REPORT_ON"):
return []
return default
class TestCollisionRejectsWholeBatch:
def test_colliding_events_persist_nothing(self, plugin_db, monkeypatch):
db, conn = plugin_db
monkeypatch.setattr("plugin.get_setting_value", _no_report_on)
plugin = make_plugin_dict(PREFIX)
events = [
make_plugin_event_row(PREFIX, "device_A", secondary_id="sec"),
make_plugin_event_row(PREFIX, "device_A", secondary_id="sec", watched1="different"),
]
process_plugin_events(db, plugin, events)
assert plugin_objects_rows(conn, PREFIX) == []
def test_collision_does_not_touch_preexisting_objects(self, plugin_db, monkeypatch):
"""A rejected batch must not modify unrelated, already-persisted
objects from prior runs either."""
db, conn = plugin_db
monkeypatch.setattr("plugin.get_setting_value", _no_report_on)
cur = conn.cursor()
seed_plugin_object(cur, PREFIX, "existing_device", watched1="val1",
status="watched-not-changed")
conn.commit()
plugin = make_plugin_dict(PREFIX)
events = [
make_plugin_event_row(PREFIX, "device_A", secondary_id="sec"),
make_plugin_event_row(PREFIX, "device_A", secondary_id="sec"),
]
process_plugin_events(db, plugin, events)
rows = plugin_objects_rows(conn, PREFIX)
assert len(rows) == 1
assert rows[0][2] == "existing_device"
def test_collision_logged_via_mylog_none(self, plugin_db, monkeypatch):
db, conn = plugin_db
monkeypatch.setattr("plugin.get_setting_value", _no_report_on)
calls = []
monkeypatch.setattr(plugin_module, "mylog", lambda level, msg: calls.append((level, msg)))
plugin = make_plugin_dict(PREFIX)
events = [
make_plugin_event_row(PREFIX, "device_A", secondary_id="sec"),
make_plugin_event_row(PREFIX, "device_A", secondary_id="sec"),
]
process_plugin_events(db, plugin, events)
assert any(level == "none" and "collision" in msg for level, msg in calls)
class TestNoCollisionBehavesAsBefore:
def test_distinct_events_persist_normally(self, plugin_db, monkeypatch):
db, conn = plugin_db
monkeypatch.setattr("plugin.get_setting_value", _no_report_on)
plugin = make_plugin_dict(PREFIX)
events = [
make_plugin_event_row(PREFIX, "device_A"),
make_plugin_event_row(PREFIX, "device_B"),
]
process_plugin_events(db, plugin, events)
rows = plugin_objects_rows(conn, PREFIX)
ids = {r[2] for r in rows}
assert ids == {"device_A", "device_B"}
def test_concatenation_boundary_shift_is_not_a_false_collision(self, plugin_db, monkeypatch):
"""idsHash hashes the (primaryId, secondaryId) pair, not their string
concatenation - ("ab", "c") and ("a", "bc") both concatenate to "abc",
which would wrongly hash identically and trip the collision guard,
rejecting a legitimate batch that has no real duplicate in it."""
db, conn = plugin_db
monkeypatch.setattr("plugin.get_setting_value", _no_report_on)
plugin = make_plugin_dict(PREFIX)
events = [
make_plugin_event_row(PREFIX, "ab", secondary_id="c"),
make_plugin_event_row(PREFIX, "a", secondary_id="bc"),
]
process_plugin_events(db, plugin, events)
rows = plugin_objects_rows(conn, PREFIX)
pairs = {(r[2], r[3]) for r in rows} # (objectPrimaryId, objectSecondaryId)
assert pairs == {("ab", "c"), ("a", "bc")}
def test_event_matching_a_preexisting_object_is_not_a_collision(self, plugin_db, monkeypatch):
"""The collision check only compares events against each other, not
against pre-existing Plugins_Objects rows - matching an existing
object is the normal "exists" path, not a collision."""
db, conn = plugin_db
monkeypatch.setattr("plugin.get_setting_value", _no_report_on)
cur = conn.cursor()
seed_plugin_object(cur, PREFIX, "device_A", watched1="val1",
status="watched-not-changed")
conn.commit()
plugin = make_plugin_dict(PREFIX)
events = [make_plugin_event_row(PREFIX, "device_A", watched1="val1")]
process_plugin_events(db, plugin, events)
rows = plugin_objects_rows(conn, PREFIX)
assert len(rows) == 1
assert rows[0][2] == "device_A"
@@ -0,0 +1,187 @@
"""
Tests for the sanitize-by-default pass in plugin_object_class.__init__.
A plugin's watchedValue*/extra/helpVal*/foreignKey fields are attacker-
influenced (parsed from network responses, headers, etc.) but persisted and
later rendered. plugin_object_class strips HTML tag-delimiter and control
characters from every mapped field by default; a column opts out via
config.json's "allow_raw_text": true, restricted to display-only types
(textarea_readonly) by
test/plugins/test_plugin_conventions.py::test_allow_raw_text_only_on_safe_types.
Run from inside the NetAlertX container - server/plugin.py isn't importable
standalone outside it (real conf/database/api imports).
pytest "test/server/test_plugin_object_field_sanitization.py" -v
"""
import os
import sys
# ---------------------------------------------------------------------------
# Path setup
# ---------------------------------------------------------------------------
INSTALL_PATH = os.getenv("NETALERTX_APP", "/app")
sys.path.extend([f"{INSTALL_PATH}/server/plugins", f"{INSTALL_PATH}/server"])
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
from db_test_helpers import make_plugin_event_row # noqa: E402
import plugin as plugin_module # noqa: E402
from plugin import plugin_object_class # noqa: E402
PREFIX = "TESTPLG"
PAYLOAD = "<img src=x onerror=alert(1)>"
STRIPPED = "img src=x onerror=alert(1)"
def _publisher_plugin(allow_raw_text_on_watched2=False):
"""Shaped like a real publisher plugin's config.json: watchedValue2
holds a raw API-response body, optionally marked allow_raw_text."""
return {
"unique_prefix": PREFIX,
"settings": [],
"database_column_definitions": [
{"column": "watchedValue1", "type": "text"},
{
"column": "watchedValue2",
"type": "textarea_readonly",
"allow_raw_text": allow_raw_text_on_watched2,
},
{"column": "extra", "type": "text"},
{"column": "helpVal1", "type": "text"},
],
}
def _watched_publisher_plugin():
"""Same shape as _publisher_plugin, but declares watchedValue1 as a WATCH
column so watchedIndxs/watchedHash actually get populated."""
plugin = _publisher_plugin()
plugin["settings"] = [{"function": "WATCH", "value": ["watchedValue1"]}]
return plugin
def _non_mac_identity_plugin():
"""A non-MAC-primaryId plugin (e.g. sync.py's GUID-keyed objects) that
declares objectPrimaryId/objectSecondaryId, so both go through the
generic sanitize loop instead of normalize_mac()."""
return {
"unique_prefix": PREFIX,
"settings": [],
"database_column_definitions": [
{"column": "objectPrimaryId", "type": "text"},
{"column": "objectSecondaryId", "type": "text"},
],
}
class TestIdsHashUsesSanitizedValues:
def test_ids_hash_reflects_sanitized_primary_id_not_raw(self):
"""Same risk class as watchedHash: idsHash drives the merge/dedup
loop in process_plugin_events(), so it must be computed from the
sanitized primaryId/secondaryId, not a bypassed raw value."""
plugin = _non_mac_identity_plugin()
clean = plugin_object_class(plugin, make_plugin_event_row(PREFIX, "hello"))
dirty = plugin_object_class(plugin, make_plugin_event_row(PREFIX, "<hello>"))
assert clean.primaryId == dirty.primaryId == "hello"
assert clean.idsHash == dirty.idsHash
class TestWatchedHashUsesSanitizedValues:
def test_watched_hash_reflects_sanitized_value_not_raw(self):
"""Two raw watched1 values that sanitize to the same text must
produce the same watchedHash - it has to be computed from self.watched1
(post-sanitization), not objDbRow's raw value."""
plugin = _watched_publisher_plugin()
clean = plugin_object_class(plugin, make_plugin_event_row(PREFIX, "id1", watched1="hello"))
dirty = plugin_object_class(plugin, make_plugin_event_row(PREFIX, "id1", watched1="<hello>"))
assert clean.watched1 == dirty.watched1 == "hello"
assert clean.watchedHash == dirty.watchedHash
def test_watched_hash_still_differs_for_genuinely_different_values(self):
plugin = _watched_publisher_plugin()
a = plugin_object_class(plugin, make_plugin_event_row(PREFIX, "id1", watched1="hello"))
b = plugin_object_class(plugin, make_plugin_event_row(PREFIX, "id1", watched1="world"))
assert a.watchedHash != b.watchedHash
class TestDefaultSanitization:
def test_watched_field_without_allow_raw_text_is_sanitized(self):
row = make_plugin_event_row(PREFIX, "id1", watched2=PAYLOAD)
obj = plugin_object_class(_publisher_plugin(), row)
assert obj.watched2 == STRIPPED
def test_extra_and_helpval_are_sanitized_by_default(self):
row = make_plugin_event_row(PREFIX, "id1", extra=PAYLOAD, help_val1=PAYLOAD)
obj = plugin_object_class(_publisher_plugin(), row)
assert obj.extra == STRIPPED
assert obj.helpVal1 == STRIPPED
def test_clean_text_passes_through_unchanged(self):
row = make_plugin_event_row(PREFIX, "id1", watched2="200 OK")
obj = plugin_object_class(_publisher_plugin(), row)
assert obj.watched2 == "200 OK"
def test_preexisting_dirty_value_is_sanitized_on_readback(self):
"""A row already persisted with an unsanitized value before this
mechanism shipped must be cleaned the next time it's read, not just
at write time - __init__ runs on every DB read, not only on insert."""
row = make_plugin_event_row(PREFIX, "id1", watched2=PAYLOAD)
obj = plugin_object_class(_publisher_plugin(), row)
assert "<" not in obj.watched2 and ">" not in obj.watched2
class TestAllowRawTextOptOut:
def test_column_with_allow_raw_text_true_is_not_sanitized(self):
row = make_plugin_event_row(PREFIX, "id1", watched2=PAYLOAD)
obj = plugin_object_class(_publisher_plugin(allow_raw_text_on_watched2=True), row)
assert obj.watched2 == PAYLOAD
def test_other_fields_still_sanitized_when_one_column_opts_out(self):
row = make_plugin_event_row(PREFIX, "id1", watched2=PAYLOAD, extra=PAYLOAD)
obj = plugin_object_class(_publisher_plugin(allow_raw_text_on_watched2=True), row)
assert obj.watched2 == PAYLOAD # opted out
assert obj.extra == STRIPPED # no opt-out on this column
class TestForeignKeySanitization:
"""foreignKey has no database_column_definitions entry of its own (no
config flag to attach an opt-out to) - always sanitized, unconditionally."""
def test_mac_shaped_foreign_key_passes_unchanged(self):
row = make_plugin_event_row(PREFIX, "id1", foreign_key="aa:bb:cc:dd:ee:ff")
obj = plugin_object_class(_publisher_plugin(), row)
assert obj.foreignKey == "aa:bb:cc:dd:ee:ff"
def test_guid_shaped_foreign_key_passes_unchanged(self):
guid = "550e8400-e29b-41d4-a716-446655440000"
row = make_plugin_event_row(PREFIX, "id1", foreign_key=guid)
obj = plugin_object_class(_publisher_plugin(), row)
assert obj.foreignKey == guid
def test_internet_sentinel_passes_unchanged(self):
row = make_plugin_event_row(PREFIX, "id1", foreign_key="internet")
obj = plugin_object_class(_publisher_plugin(), row)
assert obj.foreignKey == "internet"
def test_injection_payload_is_stripped_not_blanked(self):
row = make_plugin_event_row(PREFIX, "id1", foreign_key=PAYLOAD)
obj = plugin_object_class(_publisher_plugin(), row)
assert obj.foreignKey == STRIPPED
class TestSanitizationLogging:
def test_mylog_fires_when_value_changes(self, monkeypatch):
calls = []
monkeypatch.setattr(plugin_module, "mylog", lambda level, msg: calls.append((level, msg)))
row = make_plugin_event_row(PREFIX, "id1", watched2=PAYLOAD)
plugin_object_class(_publisher_plugin(), row)
assert any(level == "none" and "watchedValue2" in msg for level, msg in calls)
def test_mylog_does_not_fire_for_clean_values(self, monkeypatch):
calls = []
monkeypatch.setattr(plugin_module, "mylog", lambda level, msg: calls.append((level, msg)))
row = make_plugin_event_row(PREFIX, "id1", watched2="200 OK", foreign_key="aa:bb:cc:dd:ee:ff")
plugin_object_class(_publisher_plugin(), row)
assert calls == []
+19 -2
View File
@@ -1,4 +1,4 @@
from server.plugins.plugin_helper import Plugin_Object, is_mac, normalize_mac, per_item_timeout
from server.plugins.plugin_helper import Plugin_Object, is_mac, normalize_mac, per_item_timeout, sanitize_plugin_text
def test_is_mac_accepts_wildcard():
@@ -64,4 +64,21 @@ def test_watched_columns_unaffected_by_helpval_fix():
assert obj.watched1 == 0
assert obj.watched2 is False
assert obj.watched3 == ""
assert obj.watched4 is None
assert obj.watched4 is None
def test_sanitize_plugin_text_strips_tag_delimiters():
assert sanitize_plugin_text("<img src=x onerror=alert(1)>") == "img src=x onerror=alert(1)"
def test_sanitize_plugin_text_strips_control_chars_but_keeps_tab_and_newline():
assert sanitize_plugin_text("a\x00b\x1fc\td\ne\rf") == "abc\td\ne\rf"
def test_sanitize_plugin_text_passes_through_clean_text_unchanged():
text = "Living Room TV (Samsung) - " + ("x" * 2000) # no length cap
assert sanitize_plugin_text(text) == text
def test_sanitize_plugin_text_passes_none_through():
assert sanitize_plugin_text(None) is None