mirror of
https://github.com/jokob-sk/NetAlertX.git
synced 2026-10-28 03:51:08 -04:00
BE+FE: hardening
This commit is contained in:
1 parent
32eeb327c2
commit
03c8cace8a
7 files changed
+70
-33
No files matched your search
@@ -98,3 +98,6 @@ Procedural/how-to knowledge (running tests, resetting the DB, devcontainer manag
|
||||
- **No new PHP backend logic.** `front/` is being migrated away from PHP, so never add a new PHP endpoint or new server-side computation inside an existing PHP file. If a feature needs backend state or computation, add it to the Python server and expose it to the frontend through an existing read path (`app_state.json` via `query_json.php`, `table_settings.json`, a REST/GraphQL endpoint) - never re-derive logic in PHP that the Python side already knows or could easily expose. Editing existing PHP page logic (templating, bug fixes) is fine; this is about not growing the PHP surface area.
|
||||
- **A stateful UI feature (survives a reload, coordinates across tabs, reacts to a backend push) is a cross-cutting mechanism, not "just a UI feature."** Treat it like one before writing code: trace every existing consumer of the data it needs (e.g. everything that already reads `app_state.json`), not just the one file being edited. The visible surface looking small (a badge, an icon) says nothing about whether the state-propagation mechanism underneath already exists elsewhere.
|
||||
- **No JS/PHP test harness exists in `front/` — simulate before asking for a live test, every time, not after a live test fails.** Write a disposable Node/PHP script that runs the actual function(s) against realistic inputs (including inputs crossing from a different file than the one being edited) before calling a change ready to test. A diff review misses things a five-second script run catches - a renamed parameter still referenced by its old name, an ordering assumption that's wrong once two async steps are both in play.
|
||||
- **Bug and security fixes are test-first: write the failing test, run it and see it fail on the unfixed code, then fix.** Don't edit source and add tests afterward.
|
||||
- **Each user-editable config file has exactly one writer, and it must be CSRF-protected.** Read-only endpoints stay GET-only; never add a second write path. `requireCsrfToken()` reads `$_POST['csrf_token']`, so a raw-JSON-body endpoint cannot use it as-is. Loaders of user-editable JSON must tolerate a wrong top-level type and non-object entries rather than crash.
|
||||
- **Every PHP `session_start()` must be preceded by `session_set_cookie_params(['samesite' => 'Lax', 'httponly' => true])`.** A static test checks the known session-starting files, so add any new one to its list.
|
||||
@@ -42,38 +42,8 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') {
|
||||
http_response_code(400);
|
||||
echo json_encode(["error" => "Missing 'file' parameter"]);
|
||||
}
|
||||
} elseif ($_SERVER['REQUEST_METHOD'] === 'POST') {
|
||||
// Read the input JSON data
|
||||
$inputData = file_get_contents("php://input");
|
||||
$decodedData = json_decode($inputData, true);
|
||||
|
||||
if (json_last_error() !== JSON_ERROR_NONE) {
|
||||
http_response_code(400);
|
||||
echo json_encode(["error" => "Invalid JSON data"]);
|
||||
exit;
|
||||
}
|
||||
|
||||
// Check if file parameter is provided and is workflows.json
|
||||
if (!isset($_GET['file']) || $_GET['file'] !== "workflows.json") {
|
||||
http_response_code(400);
|
||||
echo json_encode(["error" => "Invalid or missing file parameter"]);
|
||||
exit;
|
||||
}
|
||||
|
||||
$file = $_GET['file'];
|
||||
$configRoot = getenv('NETALERTX_CONFIG') ?: '/data/config';
|
||||
$filePath = rtrim($configRoot, '/') . "/" . basename($file);
|
||||
|
||||
// Save new workflows.json (replace existing content)
|
||||
if (file_put_contents($filePath, json_encode($decodedData, JSON_PRETTY_PRINT))) {
|
||||
http_response_code(200);
|
||||
echo json_encode(["success" => "Workflows replaced successfully"]);
|
||||
} else {
|
||||
http_response_code(500);
|
||||
echo json_encode(["error" => "Failed to update workflows.json"]);
|
||||
}
|
||||
}
|
||||
else {
|
||||
} else {
|
||||
header('Allow: GET');
|
||||
http_response_code(405);
|
||||
echo json_encode(["error" => "Method Not Allowed"]);
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
<?php
|
||||
if (session_status() == PHP_SESSION_NONE) {
|
||||
session_set_cookie_params(['samesite' => 'Lax', 'httponly' => true]);
|
||||
session_start();
|
||||
}
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
// Start session if not already started
|
||||
if (session_status() == PHP_SESSION_NONE) {
|
||||
session_set_cookie_params(['samesite' => 'Lax', 'httponly' => true]);
|
||||
session_start();
|
||||
}
|
||||
|
||||
|
||||
@@ -52,7 +52,7 @@ class WorkflowManager:
|
||||
# -------------------------------------------------------------------------
|
||||
# Loading
|
||||
def load_workflows(self):
|
||||
"""Load workflows from workflows.json, rejecting any with invalid tokens."""
|
||||
"""Load workflows from workflows.json, skipping non-object entries and any with invalid tokens."""
|
||||
try:
|
||||
workflows_json_path = fullConfFolder + "/workflows.json"
|
||||
with open(workflows_json_path, "r") as f:
|
||||
@@ -61,8 +61,15 @@ class WorkflowManager:
|
||||
mylog("none", ["[WF] Failed to load workflows.json"])
|
||||
return []
|
||||
|
||||
if not isinstance(raw, list):
|
||||
mylog("none", ["[WF] workflows.json must contain a list of workflows - ignoring file"])
|
||||
return []
|
||||
|
||||
valid = []
|
||||
for wf in raw:
|
||||
if not isinstance(wf, dict):
|
||||
mylog("none", ["[WF] Skipping workflows.json entry that is not an object"])
|
||||
continue
|
||||
if self._validate_workflow_tokens(wf):
|
||||
valid.append(wf)
|
||||
else:
|
||||
|
||||
@@ -0,0 +1,41 @@
|
||||
"""Regression guards: workflows.json must only be writable through the CSRF-protected endpoint."""
|
||||
|
||||
import os
|
||||
import re
|
||||
|
||||
REPO_ROOT = os.path.join(os.path.dirname(__file__), "..", "..")
|
||||
QUERY_JSON = os.path.join(REPO_ROOT, "front", "php", "server", "query_json.php")
|
||||
SESSION_STARTERS = [
|
||||
os.path.join(REPO_ROOT, "front", "php", "templates", "security.php"),
|
||||
os.path.join(REPO_ROOT, "front", "php", "templates", "auth.php"),
|
||||
]
|
||||
|
||||
|
||||
def _read(path):
|
||||
"""Return the text content of a file."""
|
||||
with open(path, "r") as f:
|
||||
return f.read()
|
||||
|
||||
|
||||
def test_query_json_has_no_write_path():
|
||||
"""query_json.php is read-only: no POST handler and no file writes."""
|
||||
src = _read(QUERY_JSON)
|
||||
assert not re.search(r"REQUEST_METHOD'\]\s*===\s*'POST'", src), "POST branch must not exist"
|
||||
assert "file_put_contents" not in src, "query_json.php must not write files"
|
||||
assert "php://input" not in src, "query_json.php must not read a request body"
|
||||
|
||||
|
||||
def test_query_json_rejects_non_get_methods():
|
||||
"""Any non-GET method falls through to a 405 response."""
|
||||
src = _read(QUERY_JSON)
|
||||
assert "http_response_code(405)" in src
|
||||
|
||||
|
||||
def test_session_cookie_sets_samesite():
|
||||
"""Every file that starts the PHP session must set a SameSite attribute first."""
|
||||
for path in SESSION_STARTERS:
|
||||
src = _read(path)
|
||||
start = src.index("session_start()")
|
||||
params = src.find("session_set_cookie_params")
|
||||
assert params != -1 and params < start, f"{path}: SameSite must be set before session_start()"
|
||||
assert re.search(r"'samesite'\s*=>\s*'(Lax|Strict)'", src), f"{path}: missing SameSite Lax/Strict"
|
||||
@@ -171,6 +171,20 @@ class TestLoadWorkflowsRejectsInvalidTokens(unittest.TestCase):
|
||||
mgr = self._make_manager_loading([wf])
|
||||
self.assertEqual(len(mgr.workflows), 0)
|
||||
|
||||
def test_non_list_content_ignored(self):
|
||||
for raw in (123, "text", {"name": "x"}, None):
|
||||
mgr = self._make_manager_loading(raw)
|
||||
self.assertEqual(mgr.workflows, [])
|
||||
|
||||
def test_non_object_entries_skipped(self):
|
||||
wf = {
|
||||
"name": "Valid WF", "enabled": "Yes",
|
||||
"trigger": {"object_type": "Devices", "event_type": "update"},
|
||||
"conditions": [], "actions": []
|
||||
}
|
||||
mgr = self._make_manager_loading([1, "x", None, wf])
|
||||
self.assertEqual(len(mgr.workflows), 1)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# DeviceInstance.queryByConditions
|
||||
|
||||
Reference in new issue
Block a user