From 1a5dd9fb8e15102cd38ca0ea44b12e69a0cc8594 Mon Sep 17 00:00:00 2001 From: d3vhex Date: Mon, 14 Sep 2026 22:40:05 +0300 Subject: [PATCH] Add security keys, give security_audit a collector, and finish the console Derive the WebAuthn relying party from the request so there is nothing to configure, write the posture collector the schema had carried since the beginning with no writer, add TLS_SAN and report a certificate that predates the configured names, authenticate the automations poll so the missed-push fallback exists, stop reporting a Windows vulnerability scan that could never match as clean, move the last pages onto the shared kit with real loading and failure states, and correct the agent module reference. --- Sentora/docs/MODULES.md | 64 ++- Sentora/main.py | 105 ++++- Sentora/modules/security_audit.py | 604 +++++++++++++++++++++++++ app.py | 55 ++- docs/production-deployment.md | 124 ++++- frontend/src/components/Layout.tsx | 2 +- frontend/src/components/Sidebar.tsx | 106 ++--- frontend/src/index.css | 37 ++ frontend/src/pages/AIAnalysis.tsx | 7 +- frontend/src/pages/AccountSecurity.tsx | 6 +- frontend/src/pages/AgentDetail.tsx | 211 +++++---- frontend/src/pages/Agents.tsx | 76 +--- frontend/src/pages/Assets.tsx | 5 +- frontend/src/pages/AuditLogs.tsx | 95 ++-- frontend/src/pages/Dashboard.tsx | 8 +- frontend/src/pages/Databases.tsx | 7 +- frontend/src/pages/Deployment.tsx | 20 +- frontend/src/pages/FileIntegrity.tsx | 5 +- frontend/src/pages/LoginLogs.tsx | 2 +- frontend/src/pages/Playbooks.tsx | 7 +- frontend/src/pages/SoarHub.tsx | 2 +- frontend/src/services/api.ts | 2 + scanners/vuln.py | 23 +- tests/test_app_shadowed_names.py | 88 ++++ tests/test_auth_wiring.py | 100 ++++ tests/test_console_responsive.py | 180 ++++++++ tests/test_frontend_typecheck.py | 104 +++++ tests/test_ingest_schema_coverage.py | 31 +- tests/test_module_reference.py | 37 +- tests/test_security_audit.py | 323 +++++++++++++ tests/test_vuln_scanner.py | 42 ++ 31 files changed, 2171 insertions(+), 307 deletions(-) create mode 100644 Sentora/modules/security_audit.py create mode 100644 tests/test_app_shadowed_names.py create mode 100644 tests/test_console_responsive.py create mode 100644 tests/test_frontend_typecheck.py create mode 100644 tests/test_security_audit.py diff --git a/Sentora/docs/MODULES.md b/Sentora/docs/MODULES.md index 5bd1a9b..9f4b43e 100644 --- a/Sentora/docs/MODULES.md +++ b/Sentora/docs/MODULES.md @@ -102,10 +102,19 @@ map. - Behavior: - `info_collector` produces the package list (same logic as `inventory` but emitting the OSV input format) into `packages`. - - `find_vuln` queries `https://api.osv.dev` (or the server-proxied - air-gap mirror) for known vulnerabilities and writes - `vulnerabilities_report` rows. -- Output tables: `packages`, `vulnerabilities_report` + This is the only half that runs. + - `find_vuln` queries OSV directly and writes `vulnerabilities_report`. + **It is not scheduled and has not been for some time** — the lookup + moved to the server (`app.periodic_vuln_scan` → `scanners/vuln.py`), + so endpoints do not each spend five minutes of CPU on it and do not + each make outbound calls to a third party. The module is kept for + standalone use; nothing in `main.py` calls it. +- Output table: `packages`. `vulnerabilities_report` is written by the + server against the packages this agent ships, not by the agent. +- On Windows the server reports that scan as **skipped**, not as clean: + the package list is registry `Uninstall` display names and OSV has no + ecosystem for those. See `scan_agent` — it used to query them as NuGet + package names, which could never match. --- @@ -139,12 +148,47 @@ map. `lateral_movement` above. > All three modules in this section, plus `edr_enforcer` above, were -> documented as writing `security_audit`. None of them ever did — nothing in -> this agent has written a row to that table. The schema created it, `TABLES` -> shipped it empty every cycle, and the console showed it as NOT COLLECTED for -> the life of every install, which reads as a sensor that broke rather than one -> that was never built. It is off the shipping list; the table stays so a -> collector can be added later without a schema change. +> documented as writing `security_audit` and none of them ever did — nothing +> in the agent had written a row to that table. The schema created it, +> `TABLES` shipped it empty every cycle, and the console showed it as NOT +> COLLECTED for the life of every install, which reads as a sensor that broke +> rather than one that was never built. `security_audit` below is the +> collector that now fills it. + +### `security_audit` +- Path: `modules/security_audit.py` +- Platforms: Windows (registry, services, local accounts, firewall profiles) + / Linux (`sshd_config`, sudoers, `/etc/passwd`, file modes) +- Behavior: Hourly posture read. Reports **states**, not events — "the Guest + account is enabled" is true until somebody changes it, not something that + happened at 14:03. Read-only throughout; this module reports and `soar` + acts. +- Output table: `security_audit` (enc) for `finding` and `details` +- Three things decide the design, and each one was a bug before it was a rule: + - **Deduplication is on the finding, not the row.** `dup_fp` covers + `category` and `finding` and deliberately not `details`, because details + drift ("2 members: Administrator, pc") and a fingerprint over them + re-inserts the same finding whenever one changes. Without it, one + misconfiguration becomes one row per cycle for ever — the same failure + `send_alert` met, where a normal loopback SMB connection produced the + identical alert 288 times a day. + - **An absent registry value is the default, not an unknown.** + `DisableRealtimeMonitoring`, `AutoAdminLogon`, `UseLogonCredential` and + `SMB1` do not exist on a correctly configured machine. Reporting "could + not determine" for each gives four findings on a host with nothing wrong + with it. + - **Severity is what an attacker could do, not how alarming it sounds.** An + unquoted service path under `C:\Program Files` is MEDIUM, because + exploiting it needs write access to a directory that is admin-only on a + default install. The first run of the writable-directory check reported + `WinDefend` and `MDCoreSvc` as CRITICAL persistence — Defender's own + services, which live under `C:\ProgramData` — and a finding that accuses + the anti-malware service is enough for an operator to stop reading the + category. +- The group membership check queries by SID (`S-1-5-32-544`), not by name: + the group is `Administrators` in English and `Yöneticiler` on a Turkish + install, and matching the name is a check that silently finds nothing on a + localised Windows. ### `check_permissions` - Path: `modules/check_permissions/check_permissions.py` diff --git a/Sentora/main.py b/Sentora/main.py index 330b3d9..71be50c 100644 --- a/Sentora/main.py +++ b/Sentora/main.py @@ -92,6 +92,7 @@ def trapped_print(*args, **kwargs): from modules.find_vulns import info_collector from modules.alert.alert import main as alert_main from modules.portscanner.portscanner import main as portscanner_main +from modules.security_audit import main as security_audit_main from modules.resource_checker.disks import get_and_save_disk_info as disks from modules.edr_enforcer import main as edr_enforcer_main from modules.docker_monitor.docker_monitor import start_docker_monitor_thread @@ -495,15 +496,26 @@ def _handle_server_epoch(epoch: str) -> None: # receive and deduplicate two tables nothing ever sent it. 'software_inventory', 'network_inventory', - # `security_audit` was here and is not any more. Nothing in this agent - # ever wrote a row to it - no collector, not a broken one - so it was - # shipped empty every cycle and the console showed it permanently as - # NOT COLLECTED, which reads as a sensor that failed rather than one that - # was never built. + # `security_audit` is back, and this time something writes to it. # - # The server keeps the table and its ALLOWED_TABLES entry, so a collector - # can be added later without touching the server. What is not kept is the - # claim that this agent reports it. + # It was removed because nothing did: the table existed in both schemas, + # in the server's ingest lists and in its encrypted-field map, three + # modules were documented as producing it, and not one row was ever + # written. Shipped empty every cycle, it showed in the console as + # permanently NOT COLLECTED - a sensor that reads as broken rather than as + # never built, which is worse than an absent tab. + # + # `modules/security_audit.py` is the collector. It reports posture rather + # than events, so it deduplicates on the finding itself and not on the + # row: the same misconfiguration is re-found every cycle, and without that + # one finding becomes one row per cycle for ever. + 'security_audit', + # The rule the removal established, kept because it holds for the next + # table somebody adds: a name on this list is a claim that this agent + # reports it. An empty table here and an empty table on the server look + # identical in the console, so a table shipped with no collector behind it + # is a sensor that reads as failed rather than as absent. + # `tests/test_ingest_schema_coverage.py` now fails either way round. ] MAX_WORKERS = 6 @@ -1006,24 +1018,84 @@ def kill_old_agent_if_exists(): +_warned: set[str] = set() + + +def _warn_once(key: str, message: str) -> None: + """Say a thing the first time it happens, then stop. + + A poll that runs every few seconds turns any per-failure line into four a + second, and a log nobody can read is a log nobody reads - which is how the + 401s below sat in plain sight for the life of every install. Once is + enough to be findable; the rest is noise that hides the next problem. + """ + if key in _warned: + return + _warned.add(key) + print(message, flush=True) + + class AutomationsClient: - def __init__(self, base_url: str, timeout: int = 8): + """The poll-for-queued-work fallback, and it has to authenticate. + + These two calls carried no headers at all, so every request was anonymous + and the server answered 401 - four times every few seconds, for the life of + every install, filling the server log with them. + + The 401 is correct and the route should stay locked: unauthenticated, it + let anyone who could reach the API read the response actions queued for a + host, and POST `{"task_id": N, "status": "SUCCESS"}` against every id in + turn to mark them done. The real agent polls `WHERE status='pending'`, so + those actions then never ran while the console showed them green - a way to + switch off every autonomous response while it reports success. + + What was missed is that this is not a dead path. `call_agent_soar` queues a + pending row precisely "so an agent that missed the push can still poll for + it": this is the fallback for when the channel push does not land. Broken, + the fallback did not exist - a queued action sat `pending` for ever and + nothing ever ran it, which is exactly the state the fallback was written to + prevent. + """ + + def __init__(self, base_url: str, timeout: int = 8, agent_key: str = ""): self.base_url = base_url.rstrip("/") self.timeout = timeout + # Falls back to the module-level key so the two existing call sites + # keep working; passing it explicitly is better where the caller has it. + self.agent_key = agent_key or (AGENT_SHARED_SECRET or "") + + def _headers(self) -> dict: + return {"X-Agent-Key": self.agent_key} if self.agent_key else {} def _get_json(self, path: str, params=None): url = f"{self.base_url}{path}" - r = requests.get(url, params=params, timeout=self.timeout) + r = requests.get(url, params=params, headers=self._headers(), + timeout=self.timeout) if r.status_code == 404: return None + if r.status_code in (401, 403): + # Said once, not per poll. Silent, this is a fallback that is not + # there; four lines a second, nobody reads the log at all. + _warn_once("automations-auth", + f"[!] The server rejected this agent's key on {path} " + f"({r.status_code}). Queued response actions that miss " + f"the live channel will not be collected.") + return None r.raise_for_status() return r.json() def _post_json(self, path: str, payload: dict): url = f"{self.base_url}{path}" - r = requests.post(url, json=payload, timeout=self.timeout) + r = requests.post(url, json=payload, headers=self._headers(), + timeout=self.timeout) if r.status_code == 404: return None + if r.status_code in (401, 403): + _warn_once("automations-auth-report", + f"[!] The server rejected this agent's key reporting to " + f"{path} ({r.status_code}). Results of queued actions " + f"will not reach the console.") + return None r.raise_for_status() try: return r.json() @@ -1697,6 +1769,17 @@ def start_threads(): daemon=True ).start() + # Hourly, and that is already often for a posture check: these are states, + # not events, so re-reading them faster finds the same answers. It has to + # run more than once because the answers do change - somebody turns the + # firewall off, a new service is installed - and the deduplication means a + # cycle that finds nothing new writes nothing. + threading.Thread( + target=periodic_wrapped, + args=(security_audit_main, 3600, "security_audit"), + daemon=True + ).start() + threading.Thread( target=periodic_wrapped, args=(edr_enforcer_main, 120, "edr_enforcer"), diff --git a/Sentora/modules/security_audit.py b/Sentora/modules/security_audit.py new file mode 100644 index 0000000..ce801b0 --- /dev/null +++ b/Sentora/modules/security_audit.py @@ -0,0 +1,604 @@ +"""Posture findings: how this host is configured, not what happened on it. + +`security_audit` has existed in both schemas, in the server's ingest lists and +in its encrypted-field map since the beginning, and nothing ever wrote a row to +it. Three modules were documented as producing it and none did. So the table +shipped empty every cycle and the console showed it permanently as NOT +COLLECTED - which reads as a sensor that broke rather than one that was never +built. This is the collector. + +What belongs here, and what does not +------------------------------------ +Everything else the agent produces is an *event*: a process started, a file +changed, a log line arrived. These are **states**. "The Guest account is +enabled" is not something that happened at 14:03; it is true until somebody +changes it. + +That difference decides the whole design: + +**Deduplication is on the finding, not the row.** The same finding is re-found +every cycle, so without it one misconfiguration becomes one alert per cycle +for ever - `send_alert` learned this the hard way, where a normal loopback SMB +connection produced the identical alert 288 times a day. The fingerprint here +covers `category` and `finding` and deliberately *not* `details`, because +details carry values that drift ("2 members: Administrator, pc") and a +fingerprint over them re-inserts the same finding whenever one changes. + +**An absent setting is a secure default, not an unknown.** Most of the Windows +registry values below do not exist on a correctly configured machine: +`DisableRealtimeMonitoring`, `AutoAdminLogon`, `UseLogonCredential`, `SMB1`. +A check that reports "could not determine" for each of those produces four +findings on a hardened host, which is how a posture scanner gets switched off. +Absent means the default, and the default is the safe side in every case here. + +**Severity is what an attacker could do with it, not how alarming it sounds.** +An unquoted service path is the textbook privilege-escalation primitive and is +MEDIUM here, not HIGH, because exploiting it needs write access to a directory +that is admin-only on a default install. Calling it CRITICAL is how a finding +gets muted, and a muted finding is worse than an absent one - the console still +counts it as coverage. + +Read-only throughout. This module reports; `soar` acts. +""" +from __future__ import annotations + +import hashlib +import json +import os +import platform +import re +import stat +import subprocess +from dataclasses import dataclass +from typing import Iterable + +TABLE = "security_audit" + +#: Category values match the comment the schema has carried all along: +#: "AD", "User", "Service". `Host` covers the machine-wide settings that are +#: none of those three. +CAT_USER = "User" +CAT_SERVICE = "Service" +CAT_HOST = "Host" +CAT_AD = "AD" + + +@dataclass(frozen=True) +class Finding: + category: str + finding: str + severity: str + details: str = "" + + def fingerprint(self) -> str: + """Identity of the finding, excluding its details. + + See the module docstring: `details` drifts, and a fingerprint that + includes it turns one misconfiguration into a new row every time the + count of local administrators changes. + """ + return hashlib.sha256( + f"{self.category}|{self.finding}".encode("utf-8")).hexdigest() + + +# --------------------------------------------------------------------------- +# Pure evaluation. Every decision lives here, separately from the code that +# touches the host, so each one can be tested against a fixture rather than +# against whatever the developer's machine happens to be configured like. +# --------------------------------------------------------------------------- + +def evaluate_registry(values: dict) -> list[Finding]: + """Windows machine settings, from already-read registry values. + + `values` maps a short name to the value read, or `None` when the value is + absent. Absent is the default, and the default is secure for all of these - + see the module docstring. + """ + out: list[Finding] = [] + + # EnableLUA=0 turns UAC off entirely. Absent means on. + if values.get("EnableLUA") == 0: + out.append(Finding( + CAT_HOST, "User Account Control is disabled", "HIGH", + "EnableLUA=0. Every elevation prompt is skipped, so any code that " + "runs as an administrator account runs elevated without asking.")) + + # fDenyTSConnections=0 means RDP is accepting connections. Not a + # misconfiguration by itself - plenty of hosts are meant to - so this is + # LOW and phrased as a fact rather than a fault. + if values.get("fDenyTSConnections") == 0: + out.append(Finding( + CAT_HOST, "Remote Desktop is enabled", "LOW", + "fDenyTSConnections=0. Worth knowing on a workstation; expected on " + "a jump host. Check that Network Level Authentication is on.")) + + if values.get("DisableRealtimeMonitoring") == 1: + out.append(Finding( + CAT_HOST, "Defender real-time protection is disabled", "CRITICAL", + "DisableRealtimeMonitoring=1. This is set by policy or by " + "somebody with administrator rights; it is also the first thing " + "an intruder does, and it is how the next stage goes unscanned.")) + + if values.get("SMB1") == 1: + out.append(Finding( + CAT_HOST, "SMBv1 is enabled", "HIGH", + "SMB1=1 on LanmanServer. The protocol has no message signing " + "worth the name and is the transport EternalBlue used.")) + + if values.get("AutoAdminLogon") in (1, "1"): + out.append(Finding( + CAT_USER, "Automatic logon is configured", "HIGH", + "AutoAdminLogon=1. The account's password is stored in the " + "registry under Winlogon\\DefaultPassword, readable by anyone who " + "can read HKLM - which includes every local administrator and " + "anything running as one.")) + + if values.get("UseLogonCredential") == 1: + out.append(Finding( + CAT_USER, "WDigest is caching plaintext credentials", "CRITICAL", + "UseLogonCredential=1. This puts cleartext passwords back in LSASS " + "memory, which is exactly what a credential dump reads. The " + "default on this Windows version is off; something set it.")) + + if values.get("LimitBlankPasswordUse") == 0: + out.append(Finding( + CAT_USER, "Blank passwords may be used remotely", "HIGH", + "LimitBlankPasswordUse=0. An account with no password can " + "authenticate over the network rather than only at the console.")) + + return out + + +def evaluate_firewall(profiles: dict) -> list[Finding]: + """One finding per disabled profile, named. + + Not one finding saying "the firewall is off": the profiles are independent + and which one is off changes what it means. Public off on a laptop is a + different problem from Domain off on a server. + """ + off = sorted(name for name, enabled in (profiles or {}).items() + if enabled is False) + if not off: + return [] + return [Finding( + CAT_HOST, f"Host firewall is disabled on the {name} profile", + "HIGH" if name in ("Public", "Domain") else "MEDIUM", + f"The {name} profile is not filtering. Inbound connections reach " + f"whatever is listening, and this platform's own port scan is the only " + f"thing that will tell you what that is.") + for name in off] + + +#: A path is only hijackable if somebody who is not already an administrator +#: can write to one of the directories Windows would try. On a default install +#: `C:\` and `C:\Program Files` are not writable, so the finding is about a +#: latent weakness rather than an open door - hence MEDIUM, and hence saying so. +_UNQUOTED_SAFE_PREFIXES = ("c:\\windows\\", "c:\\program files\\", + "c:\\program files (x86)\\") + + +def evaluate_service_paths(services: Iterable[dict]) -> list[Finding]: + """Unquoted service binary paths containing a space. + + Windows resolves `C:\\Program Files\\App\\svc.exe` by trying + `C:\\Program.exe` first, then `C:\\Program Files\\App\\svc.exe`. A writable + directory anywhere along that sequence is a way to have your binary started + as whatever the service runs as. + """ + out: list[Finding] = [] + for svc in services or []: + path = (svc.get("path") or "").strip() + name = svc.get("name") or "?" + if not path or path.startswith('"'): + continue + # The first token has to contain a space before the executable for the + # ambiguity to exist at all. + head = path.split(".exe")[0] + if " " not in head or "\\" not in head: + continue + low = path.lower() + privileged = not low.startswith(_UNQUOTED_SAFE_PREFIXES) + out.append(Finding( + CAT_SERVICE, f"Service binary path is unquoted: {name}", + "HIGH" if privileged else "MEDIUM", + f"ImagePath={path}. Windows tries each space-separated prefix in " + f"turn, so a writable directory along that sequence lets somebody " + f"substitute the binary." + + ("" if privileged else + " The path is under a directory only administrators can write " + "to on a default install, which is what keeps this latent."))) + return out + + +def evaluate_service_accounts(services: Iterable[dict]) -> list[Finding]: + """Services running as SYSTEM from a directory anyone can write. + + The combination is the finding. A binary in `%TEMP%` is unusual; a binary + in `%TEMP%` started as SYSTEM at every boot is persistence. + """ + # `\programdata\` was in this list and is deliberately gone. That + # directory's root is writable by users, which is what makes it the + # textbook example - but the vendor subdirectories under it are not, and + # Defender's own services live at + # `C:\ProgramData\Microsoft\Windows Defender\Platform\...`. The first run + # of this check reported WinDefend, WdNisSvc and MDCoreSvc as CRITICAL + # persistence, which is the worst false positive available: it accuses the + # anti-malware service of being the malware, and one of those is enough + # for an operator to stop reading the category. + # + # Deciding it properly means asking whether a non-administrator can write + # to that specific directory - an ACL query per service, too expensive for + # every cycle. So the list is narrowed to directories that are writable by + # design and have no legitimate business holding a service binary. A + # binary under a vendor directory with wrong ACLs is missed; that is a + # stated limit rather than a silent one. + writable = ("\\temp\\", "\\users\\public\\", + "\\appdata\\", "\\windows\\tasks\\") + out: list[Finding] = [] + for svc in services or []: + path = (svc.get("path") or "").strip().strip('"').lower() + account = (svc.get("account") or "").lower() + name = svc.get("name") or "?" + if not path or not any(w in path for w in writable): + continue + privileged = any(a in account for a in + ("localsystem", "system", "administrator")) + out.append(Finding( + CAT_SERVICE, f"Service binary is in a writable directory: {name}", + "CRITICAL" if privileged else "HIGH", + f"ImagePath={svc.get('path')} runs as {svc.get('account') or 'an unknown account'}. " + f"Services start themselves and survive a reboot, which is why " + f"persistence keeps coming back to them.")) + return out + + +def evaluate_local_accounts(users: Iterable[dict], admin_members: Iterable[str]) -> list[Finding]: + """Enabled accounts, their password policy, and who is an administrator.""" + out: list[Finding] = [] + users = list(users or []) + + for user in users: + name = user.get("name") or "?" + if not user.get("enabled"): + continue + if name.lower() == "guest": + out.append(Finding( + CAT_USER, "The Guest account is enabled", "HIGH", + "Guest authenticates with no password and is a member of " + "Everyone. It is disabled by default and there is rarely a " + "reason to change that.")) + if user.get("password_expires") is False: + out.append(Finding( + CAT_USER, f"Password never expires: {name}", "LOW", + f"{name} is enabled and its password has no expiry. Ordinary " + f"for a service account, worth knowing for a person's.")) + if user.get("password_required") is False: + # Worded carefully, and downgraded from HIGH after checking what + # the flag means. `PasswordRequired=False` says the account is + # *permitted* to have a blank password, not that it has one - this + # host's `pc` account has the flag clear and a password set since + # July. The first draft claimed "can authenticate with an empty + # password", which is a different and much louder statement, and a + # finding that overstates is one an operator learns to dismiss + # along with the next one. + out.append(Finding( + CAT_USER, f"Password is not required: {name}", "MEDIUM", + f"{name} is permitted to have a blank password. It may well " + f"have one set now; the point is that nothing stops it being " + f"cleared, and `LimitBlankPasswordUse=0` would then make it " + f"usable over the network rather than only at the console.")) + + members = sorted(set(admin_members or [])) + if len(members) > 2: + # Not a fault, a fact worth surfacing: every extra administrator is + # another account whose compromise is a full compromise. + out.append(Finding( + CAT_USER, "More than two local administrators", "LOW", + f"{len(members)} members: {', '.join(members)}. Each one is a " + f"full compromise of this host if its credentials are taken.")) + return out + + +def evaluate_sshd_config(text: str) -> list[Finding]: + """Effective sshd settings, last directive winning. + + sshd takes the *first* occurrence of most keywords, which is the opposite + of what a naive reader assumes - but a commented-out default followed by an + explicit setting is the common shape, so the parse has to ignore comments + and then take the first live value. + """ + out: list[Finding] = [] + seen: dict[str, str] = {} + for raw in (text or "").splitlines(): + line = raw.split("#", 1)[0].strip() + if not line: + continue + parts = line.split(None, 1) + if len(parts) != 2: + continue + key = parts[0].lower() + if key not in seen: # first occurrence wins, as sshd does + seen[key] = parts[1].strip().lower() + + if seen.get("permitrootlogin") in ("yes", "without-password", "prohibit-password"): + risky = seen["permitrootlogin"] == "yes" + out.append(Finding( + CAT_USER, "SSH permits root login", + "HIGH" if risky else "LOW", + f"PermitRootLogin {seen['permitrootlogin']}. " + + ("Root can authenticate with a password, so every failed guess " + "is against the account that needs no privilege escalation." + if risky else + "Key-only, which is defensible; it still means the account " + "everybody can name is directly reachable."))) + + if seen.get("passwordauthentication") == "yes": + out.append(Finding( + CAT_USER, "SSH accepts password authentication", "MEDIUM", + "PasswordAuthentication yes. Every password on this host is " + "reachable from anywhere the port is.")) + + if seen.get("permitemptypasswords") == "yes": + out.append(Finding( + CAT_USER, "SSH accepts empty passwords", "CRITICAL", + "PermitEmptyPasswords yes. An account with no password is a login " + "with no credential.")) + return out + + +_NOPASSWD = re.compile(r"^\s*([^#\s][^\s]*)\s+.*NOPASSWD\s*:", re.I) + + +def evaluate_sudoers(text: str) -> list[Finding]: + """NOPASSWD grants, which turn a shell into a root shell.""" + grants = [] + for raw in (text or "").splitlines(): + line = raw.split("#", 1)[0] + m = _NOPASSWD.match(line) + if m: + grants.append(m.group(1)) + if not grants: + return [] + return [Finding( + CAT_USER, "sudo is granted without a password", "HIGH", + f"NOPASSWD for: {', '.join(sorted(set(grants)))}. Anything that can " + f"run as one of these becomes root without needing the password - " + f"which includes a web shell, a hijacked session, or a stolen key.")] + + +def evaluate_passwd(text: str) -> list[Finding]: + """UID 0 accounts other than root.""" + extra = [] + for raw in (text or "").splitlines(): + parts = raw.strip().split(":") + if len(parts) < 3: + continue + name, _, uid = parts[0], parts[1], parts[2] + if uid == "0" and name != "root": + extra.append(name) + if not extra: + return [] + return [Finding( + CAT_USER, "An account other than root has UID 0", "CRITICAL", + f"{', '.join(extra)} share root's user id, so they are root - with a " + f"different name, which is the point of doing it.")] + + +def evaluate_file_modes(modes: dict) -> list[Finding]: + """Permissions on files whose readability is the whole control.""" + out: list[Finding] = [] + for path, mode in (modes or {}).items(): + if mode is None: + continue + world = stat.S_IMODE(mode) & 0o007 + if path.endswith("shadow") and world: + out.append(Finding( + CAT_HOST, f"{path} is world-readable", "CRITICAL", + f"mode {oct(stat.S_IMODE(mode))}. Every password hash on this " + f"host is readable by every account on it.")) + elif world & 0o002: + out.append(Finding( + CAT_HOST, f"{path} is world-writable", "HIGH", + f"mode {oct(stat.S_IMODE(mode))}.")) + return out + + +# --------------------------------------------------------------------------- +# Host access. Thin on purpose: gather, hand to an evaluator above. +# --------------------------------------------------------------------------- + +_PS_QUERY = r""" +$ErrorActionPreference = 'SilentlyContinue' +$users = Get-LocalUser | ForEach-Object { + [pscustomobject]@{ + name = $_.Name; enabled = [bool]$_.Enabled + password_expires = ($null -ne $_.PasswordExpires) + password_required = [bool]$_.PasswordRequired + } +} +# By SID, not by name. This group is called "Administrators" in English and +# "Yoneticiler" on a Turkish install, among others - matching the name is a +# check that silently finds nothing on a localised Windows. +$admins = Get-LocalGroupMember -SID 'S-1-5-32-544' | ForEach-Object { $_.Name } +$fw = @{} +Get-NetFirewallProfile | ForEach-Object { $fw[$_.Name] = [bool]$_.Enabled } +[pscustomobject]@{ users = $users; admins = $admins; firewall = $fw } | + ConvertTo-Json -Depth 4 -Compress +""" + + +def _windows_facts() -> dict: + """One PowerShell call for the things `winreg` and `psutil` cannot see.""" + try: + proc = subprocess.run( + ["powershell", "-NoProfile", "-NonInteractive", "-Command", _PS_QUERY], + capture_output=True, timeout=90, + ) + # Decoded here rather than with `text=True`. That decodes using the + # console code page - cp1254 on this Turkish install - and an account + # or service name outside it raises UnicodeDecodeError inside + # subprocess's own reader thread, where the caller cannot catch it: the + # traceback prints and the call returns empty output with no exception. + # A localised Windows is the normal case, not the edge one. + raw = (proc.stdout or b"").decode("utf-8", errors="replace") + return json.loads(raw or "{}") or {} + except Exception as exc: + print(f"[security_audit] local account query failed: {exc}", flush=True) + return {} + + +_REG_READS = [ + # (short name, hive-relative key, value name) + ("EnableLUA", r"SOFTWARE\Microsoft\Windows\CurrentVersion\Policies\System", "EnableLUA"), + ("LimitBlankPasswordUse", r"SYSTEM\CurrentControlSet\Control\Lsa", "LimitBlankPasswordUse"), + ("fDenyTSConnections", r"System\CurrentControlSet\Control\Terminal Server", "fDenyTSConnections"), + ("SMB1", r"SYSTEM\CurrentControlSet\Services\LanmanServer\Parameters", "SMB1"), + ("AutoAdminLogon", r"SOFTWARE\Microsoft\Windows NT\CurrentVersion\Winlogon", "AutoAdminLogon"), + ("UseLogonCredential", r"SYSTEM\CurrentControlSet\Control\SecurityProviders\WDigest", "UseLogonCredential"), + ("DisableRealtimeMonitoring", r"SOFTWARE\Policies\Microsoft\Windows Defender\Real-Time Protection", "DisableRealtimeMonitoring"), +] + + +def _windows_registry() -> dict: + """Read each value, or None when it is absent. + + None and 0 are different answers and the evaluator relies on it: absent + means the Windows default, which is the secure side for all of these. + """ + import winreg + + out: dict = {} + for short, key, name in _REG_READS: + out[short] = None + try: + with winreg.OpenKey(winreg.HKEY_LOCAL_MACHINE, key) as handle: + value, _kind = winreg.QueryValueEx(handle, name) + out[short] = int(value) if str(value).isdigit() else value + except FileNotFoundError: + pass # absent: the default + except OSError: + pass + return out + + +def _windows_services() -> list[dict]: + try: + import psutil + except ImportError: + return [] + out = [] + for svc in psutil.win_service_iter(): + try: + info = svc.as_dict() + out.append({"name": info.get("name"), + "path": info.get("binpath"), + "account": info.get("username")}) + except Exception: + continue + return out + + +def _read(path: str) -> str: + try: + with open(path, "r", encoding="utf-8", errors="replace") as fh: + return fh.read() + except OSError: + return "" + + +def _linux_sudoers() -> str: + text = _read("/etc/sudoers") + drop_in = "/etc/sudoers.d" + if os.path.isdir(drop_in): + for name in sorted(os.listdir(drop_in)): + text += "\n" + _read(os.path.join(drop_in, name)) + return text + + +def _modes(paths: Iterable[str]) -> dict: + out = {} + for path in paths: + try: + out[path] = os.stat(path).st_mode + except OSError: + out[path] = None + return out + + +def collect() -> list[Finding]: + """Every finding for this host, whichever platform it is.""" + findings: list[Finding] = [] + + if platform.system().lower() == "windows": + findings += evaluate_registry(_windows_registry()) + services = _windows_services() + findings += evaluate_service_paths(services) + findings += evaluate_service_accounts(services) + facts = _windows_facts() + findings += evaluate_firewall(facts.get("firewall") or {}) + findings += evaluate_local_accounts(facts.get("users") or [], + facts.get("admins") or []) + else: + findings += evaluate_sshd_config(_read("/etc/ssh/sshd_config")) + findings += evaluate_sudoers(_linux_sudoers()) + findings += evaluate_passwd(_read("/etc/passwd")) + findings += evaluate_file_modes( + _modes(["/etc/shadow", "/etc/passwd", "/etc/sudoers"])) + + return findings + + +def main() -> None: + """Write what is new, and say how many were already known. + + Deduplicated against what is already stored rather than inserted blindly: + these are states, so every row would otherwise be rewritten on every + cycle. `dup_fp` is set by the module, not by `insert_record_enc`, because + the identity of a finding excludes its details - see `Finding.fingerprint`. + """ + from modules.enc_db import insert_record_enc + from modules.db import fetch_where + + findings = collect() + if not findings: + print("[security_audit] no findings", flush=True) + return + + try: + rows = fetch_where(TABLE, "dup_fp IS NOT NULL", ()) + known = {r["dup_fp"] for r in rows or [] if r.get("dup_fp")} + except Exception as exc: + # A first run before the table exists, or a schema that has not caught + # up. Writing the findings matters more than deduplicating them. + print(f"[security_audit] could not read existing findings ({exc}); " + f"writing without deduplication", flush=True) + known = set() + + written = 0 + for finding in findings: + fp = finding.fingerprint() + if fp in known: + continue + try: + insert_record_enc(TABLE, { + "category": finding.category, + "finding": finding.finding, + "severity": finding.severity, + "details": finding.details, + "dup_fp": fp, + "sent": False, + }) + known.add(fp) + written += 1 + except Exception as exc: + print(f"[security_audit] {finding.finding!r} not stored: " + f"{type(exc).__name__}: {exc}", flush=True) + + print(f"[security_audit] {len(findings)} finding(s), {written} new", flush=True) + + +if __name__ == "__main__": + for f in collect(): + print(f"{f.severity:<8} [{f.category}] {f.finding}") + if f.details: + print(f" {f.details}") diff --git a/app.py b/app.py index 28587a2..fca1854 100644 --- a/app.py +++ b/app.py @@ -1246,8 +1246,20 @@ async def audit_log(request, action: str, resource: str, details: str = ""): # before the agent polled them, so they never ran and the UI showed them # green. They authenticate now — see _require_agent. "download_agent", "register_agent", "agent_bootstrap", - # See the path list above: the pending token is the credential. + # See the path list below: the pending token is the credential. "complete_second_factor", + # The same login finished with a security key. Public in exactly the sense + # `complete_second_factor` is: no browser session exists yet, and the + # credential is the pending token minted moments earlier by a correct + # password — single use, five minutes, able to do one thing. + # + # These two were added to `_PUBLIC_EXACT_PATHS` first, which is the wrong + # list and says so in its own comment: it is a path-shaped *mirror* used + # only as a backstop when the router cannot resolve a handler. `authenticate` + # checks this set, matches on handler name, and returns 401 long before the + # backstop is consulted — so both routes answered "Authentication required" + # on the login page, in half a millisecond, without running. + "webauthn_login_begin", "webauthn_login_finish", # The CA certificate, and only the certificate. It is the half of the pair # whose purpose is to be handed out, an installer needs it before it holds # any credential, and publishing it grants nothing - it lets a client check @@ -1294,6 +1306,19 @@ async def audit_log(request, action: str, resource: str, details: str = ""): # password, single-use, five-minute lifetime, and able to do exactly one # thing. Gating it on a session would make the second factor unreachable. "/login/2fa", + # The same login, finished with a security key instead of a code, and + # public for exactly the same reason: the pending token is the credential + # and there is no session to require. + # + # These were added to the auth-wiring test's list of session-only routes + # and not here, which reads as the same decision and is not: that list + # says "no permission check is expected", this one says "the session + # middleware must not reject it first". So the middleware answered 401 + # before either handler ran, and the console showed "Authentication + # required" on a login page - where the whole point is that the operator + # is not authenticated yet. + "/login/2fa/webauthn/begin", + "/login/2fa/webauthn/finish", # The CA *certificate*, so an agent can verify a self-signed server. # Public on purpose: a certificate is the half of the pair meant to be # handed out, and an installer needs it before it has any credential. @@ -3181,6 +3206,30 @@ async def get_packages(request, agent): encrypted_fields=ENCRYPTED_FIELDS_MAP["packages"] ) +@require_permission("read_telemetry") +@app.route("//security_audit") +async def get_security_audit(request, agent): + """Posture findings: how the host is configured, not what happened on it. + + The last link in a chain that was broken at every point. The table existed + in both schemas and in the ingest lists with nothing writing to it; the + collector was added and the rows arrived; and there was still no route, so + the data reached the server, stored correctly, decryptable, and could not + be looked at. Each of those is the same failure one step further along. + + `finding` and `details` are encrypted at rest - they name the + misconfiguration and where it is, which is a map for anyone who reads the + database - so this goes through the decrypting reader. `category` and + `severity` are plaintext on purpose: the console filters and sorts on them, + and neither says anything a reader did not already know from the table's + name. + """ + return await stream_from_db_dec( + "security_audit", agent, connect_db_for_agent, + encrypted_fields=ENCRYPTED_FIELDS_MAP["security_audit"], + ) + + @require_permission("read_telemetry") @app.route("//ai_logs") async def get_ai_logs(request, agent): @@ -8141,7 +8190,7 @@ async def webauthn_register_begin(request): ), ) return sanic_json({"status": "success", - "options": json.loads(lib["lib"].options_to_json(options))}) + "options": pyjson.loads(lib["lib"].options_to_json(options))}) @app.post("/api/2fa/webauthn/register/finish") @@ -8328,7 +8377,7 @@ async def webauthn_login_begin(request): user_verification=lib["UserVerification"].PREFERRED, ) return sanic_json({"status": "success", - "options": json.loads(lib["lib"].options_to_json(options))}) + "options": pyjson.loads(lib["lib"].options_to_json(options))}) @app.post("/login/2fa/webauthn/finish") diff --git a/docs/production-deployment.md b/docs/production-deployment.md index 0e71b75..5ba9dd2 100644 --- a/docs/production-deployment.md +++ b/docs/production-deployment.md @@ -519,7 +519,100 @@ Agents pin nothing: they verify against the system trust store plus `server_ca` if one is set. With a certificate from a public CA, renewal needs nothing on the endpoints at all. -### 3.8 Verifying it, rather than assuming it +### 3.8 Giving the deployment a name + +Everything above works with the default `TLS_CN=localhost`, and for a single +machine that is the whole story. Two things need a real hostname, and both are +decisions rather than defaults: + +- **Operators reaching the console from another machine.** At + `https://10.0.0.5:8000` the browser reports a name mismatch *on top of* the + untrusted-CA warning — two errors that look like one, and the second is the + one people learn to click through. +- **Security keys.** WebAuthn's relying-party ID must be a domain. Browsers + reject an IP address outright, so a console reached at an address is one + where a key can never be registered. One-time codes are unaffected. See §6.2. + +Nothing else in the platform requires it. Agents are happy with an address. + +#### What to change + +**1. Pick a name and make it resolve.** It does not have to be public or have +a public certificate — `sentora.corp.local`, an internal A record, or a +`hosts` entry on each operator's machine are all fine. It does have to be the +name people actually type, because that is what the browser signs over. + +**2. Set both values in `.env`:** + +```ini +TLS_ENABLED=1 +TLS_CN=sentora.corp.local +TLS_SAN=sentora.corp.local,10.0.0.5 +``` + +`TLS_SAN` is comma separated and takes hostnames and addresses alike. An entry +that parses as an address becomes an **IP SAN**, because a bare address listed +as a DNS name matches nothing. `localhost`, `127.0.0.1` and `::1` are always +included, so the machine itself keeps working. + +Keep the address in the list. Dropping it breaks the console for anyone still +using it, and agents enrolled against `https://10.0.0.5` keep verifying that +name until they are re-enrolled. + +**3. Regenerate the material.** This is the step that is easy to miss: + +```bash +docker compose exec app python certs/generate_certs.py --force \ + --cn sentora.corp.local --san sentora.corp.local,10.0.0.5 \ + --out /app/data/certs +docker compose restart app ingest +``` + +Generation is idempotent on purpose — a new identity on every restart would +break every agent that pinned the CA — so **editing `.env` alone changes +nothing**. The server says so at startup rather than leaving you to find it: + +``` +[tls] The certificate at /app/data/certs/server.crt does not cover +sentora.corp.local - it was generated before those names were configured, and +TLS material is never regenerated on top of itself. ... run +`python certs/generate_certs.py --force --cn sentora.corp.local --san ...` +``` + +**4. `--force` issues a new CA as well.** Every agent holding the old +`rootCA.crt` will refuse the new server and stop sending — see §3.7. Either +distribute the new CA to the endpoints first, or re-enrol them, which does it +for you. On a small fleet, re-enrolling is less error-prone. + +**5. Point the agents at the name**, so the certificate they verify matches +the host they dialled: + +```json +{ "server_url": "https://sentora.corp.local:8000" } +``` + +A re-enrolled agent gets this from the installer. An existing one needs its +config edited and a restart. + +**6. Check it, rather than assume it** (§3.9 has the rest): + +```bash +openssl s_client -connect sentora.corp.local:8000 -servername sentora.corp.local \ + /dev/null | openssl x509 -noout -subject -ext subjectAltName +``` + +The SAN must list the name you typed. Then open the console at that name and +check **My Security** — it reports whether a security key can be registered +from where the browser is standing, and why not when it cannot. + +#### If you skip all of this + +Nothing breaks. The console stays on `localhost`, agents keep reporting, +two-factor keeps working with one-time codes, and the browser keeps warning +about the CA. The only thing you do not get is a security key, and the console +says so in place of offering one. + +### 3.9 Verifying it, rather than assuming it Run these after enabling TLS. Every one of them found something the first time. @@ -572,7 +665,7 @@ docker compose logs ingest | grep "in the clear on port" the failure this section keeps warning about. Check it there rather than inferring health from row counts. -### 3.9 Why not mTLS, and what it would take +### 3.10 Why not mTLS, and what it would take Client certificates were considered for agent authentication and deliberately not implemented. The reasoning, so it does not have to be reconstructed later: @@ -735,6 +828,33 @@ at risk. | `GET /api/ai-insights/all` | AI worker output | | `:15672` | RabbitMQ queue depth, throughput, dead letters | +#### Posture findings + +`security_audit` carries how each host is *configured*, as opposed to what +happened on it: UAC off, SMBv1 on, a firewall profile disabled, an unquoted +service path, SSH permitting root. Read-only — the agent reports, SOAR acts. + +These are states, not events, so the table behaves differently from the rest: + +- **A finding appears once and stays.** It is deduplicated on the finding + itself, not on the row, so a cycle that finds nothing new writes nothing. A + row that stops being true is not removed automatically; clear it from the + Databases view once the underlying setting is fixed, or the finding + reappears only if it recurs. +- **An empty table is a real answer here.** Unlike the telemetry tables, where + empty usually means something is broken, a host with nothing misconfigured + produces no rows. The collector logs `[security_audit] N finding(s), M new` + every cycle, which is how you tell "nothing wrong" from "not running". +- **Severity is what an attacker could do with it.** An unquoted service path + under `C:\Program Files` is MEDIUM, not HIGH: exploiting it needs write + access to a directory only administrators have. Findings that overstate are + findings operators stop reading. + +The table existed empty for the life of every earlier install — the schema had +it, ingest accepted it, and no collector was ever written — so a deployment +upgraded from that state will start seeing rows for the first time. They are +not new problems; they are the first look. + ### 5.2 Boot-time assertions worth alerting on The server prints these at startup. They are cheap to scrape and each one diff --git a/frontend/src/components/Layout.tsx b/frontend/src/components/Layout.tsx index c42877e..38b87a5 100644 --- a/frontend/src/components/Layout.tsx +++ b/frontend/src/components/Layout.tsx @@ -79,7 +79,7 @@ const Layout: React.FC<{ children: React.ReactNode }> = ({ children }) => { /> )} - +
= ({ to, icon, label }) => { @@ -34,7 +44,7 @@ const SidebarLink: React.FC<{ to: string, icon: React.ReactNode, label: string } ); }; -const Sidebar: React.FC = ({ isOpen }) => { +const Sidebar: React.FC = ({ isOpen, isMobile }) => { const user = authService.getUser(); const [showPasswordModal, setShowPasswordModal] = useState(false); @@ -93,7 +103,7 @@ const Sidebar: React.FC = ({ isOpen }) => { display: 'flex', flexDirection: 'column', height: '100vh', - position: window.innerWidth <= 1024 ? 'fixed' : 'sticky', + position: isMobile ? 'fixed' : 'sticky', top: 0, left: 0, flexShrink: 0, @@ -101,7 +111,7 @@ const Sidebar: React.FC = ({ isOpen }) => { overflow: 'hidden', zIndex: 999, boxShadow: isOpen ? '4px 0 24px rgba(0,0,0,0.2)' : 'none', - visibility: isOpen || window.innerWidth > 1024 ? 'visible' : 'hidden' + visibility: isOpen || !isMobile ? 'visible' : 'hidden' }}>
= ({ isOpen }) => { {showPasswordModal && ( -
-
-
-

Change Password

- + setShowPasswordModal(false)} + > +
+ + setPasswordData({ ...passwordData, current_password: e.target.value })} + required + autoFocus + /> + + + setPasswordData({ ...passwordData, new_password: e.target.value })} + required + /> + + + setPasswordData({ ...passwordData, confirm_password: e.target.value })} + required + /> + +
+ setShowPasswordModal(false)}>Cancel + Update
- - -
- - setPasswordData({...passwordData, current_password: e.target.value})} - required - style={{ backgroundColor: 'var(--bg-color)', border: '1px solid var(--border-color)', padding: '12px', borderRadius: '8px', color: 'white' }} - /> -
-
- - setPasswordData({...passwordData, new_password: e.target.value})} - required - style={{ backgroundColor: 'var(--bg-color)', border: '1px solid var(--border-color)', padding: '12px', borderRadius: '8px', color: 'white' }} - /> -
-
- - setPasswordData({...passwordData, confirm_password: e.target.value})} - required - style={{ backgroundColor: 'var(--bg-color)', border: '1px solid var(--border-color)', padding: '12px', borderRadius: '8px', color: 'white' }} - /> -
- -
- - -
-
-
-
+ + )} ); diff --git a/frontend/src/index.css b/frontend/src/index.css index b80f49f..bcdf79f 100644 --- a/frontend/src/index.css +++ b/frontend/src/index.css @@ -145,6 +145,43 @@ body { gap: 24px; } +/* A main column and a narrower one beside it. + * + * Six pages wrote this by hand as `gridTemplateColumns: '300px 1fr'` or + * similar, and an inline style cannot carry a media query - so every one of + * them kept two columns at 400px wide, where the sidebar and the table are + * each too narrow to read and the body scrolls sideways. + * + * `auto-fit` does not solve this the way it solves `.responsive-grid`: these + * layouts are deliberately asymmetric, and auto-fit makes both columns the + * same width. Hence a class with an actual breakpoint, which is the thing an + * inline style cannot express. + * + * `minmax(0, …)` on the main column and not `2.4fr` alone: a grid item + * defaults to `min-width: auto`, so a wide table or a long unbroken string + * pushes the column past its share instead of scrolling inside it. That is + * the "the sidebar drifts" bug, and it is always this. + */ +.split-grid { + display: grid; + grid-template-columns: minmax(0, 2.4fr) minmax(260px, 1fr); + gap: 24px; + align-items: start; +} + +/* Sidebar first, main second - Assets, FileIntegrity, Databases. */ +.split-grid--aside-first { + grid-template-columns: minmax(220px, 300px) minmax(0, 1fr); +} + +@media (max-width: 1100px) { + .split-grid, + .split-grid--aside-first { + grid-template-columns: 1fr; + gap: 16px; + } +} + @media (max-width: 1280px) { .responsive-grid { grid-template-columns: repeat(auto-fit, minmax(280px, 1fr)); diff --git a/frontend/src/pages/AIAnalysis.tsx b/frontend/src/pages/AIAnalysis.tsx index 7706951..040fd74 100644 --- a/frontend/src/pages/AIAnalysis.tsx +++ b/frontend/src/pages/AIAnalysis.tsx @@ -266,12 +266,7 @@ const AIAnalysis: React.FC = () => { } /> -
+
diff --git a/frontend/src/pages/AccountSecurity.tsx b/frontend/src/pages/AccountSecurity.tsx index ffb4e9c..436e8a6 100644 --- a/frontend/src/pages/AccountSecurity.tsx +++ b/frontend/src/pages/AccountSecurity.tsx @@ -283,7 +283,7 @@ const AccountSecurity: React.FC = () => { padding: 'var(--space-3)', borderRadius: 'var(--radius-md)', background: 'var(--bg-color)', border: '1px solid var(--border-color)', color: 'var(--text-primary)', fontFamily: 'monospace', - letterSpacing: '0.25em', width: '160px', + letterSpacing: '0.25em', width: '100%', maxWidth: '160px', }} />
)} + {/* `.card` already carries the background, border, radius and padding + this used to set inline, including a media query that drops the + padding to 16px under 768px - which is what the inline + `window.innerWidth > 768` was reimplementing, and getting wrong: + that value is read once per render, so it never followed a + resize. */} {activeTab === 'config' && ( -
768 ? '32px' : '16px' }}> +
setConfigType('rules')} /> @@ -752,6 +767,7 @@ const AgentDetail: React.FC = () => { )} {activeTab === 'ai' && } {activeTab === 'soar' && } + {activeTab === 'posture' && } {activeTab === 'system' && }
@@ -830,7 +846,7 @@ const OverviewTab: React.FC<{ data: any }> = ({ data }) => { onMouseOver={e => e.currentTarget.style.backgroundColor = 'rgba(255,255,255,0.03)'} onMouseOut={e => e.currentTarget.style.backgroundColor = 'rgba(255,255,255,0.015)'} > -
{p.timestamp || event.timestamp}
+
{p.timestamp || event.timestamp}
[{p.source || 'LOG'}] {p.message} @@ -1107,7 +1123,7 @@ const TableTab: React.FC<{ placeholder="Search data..." value={searchTerm} onChange={(e) => setSearchTerm(e.target.value)} - style={{ backgroundColor: 'var(--bg-color)', border: '1px solid var(--border-color)', borderRadius: '6px', padding: '6px 12px 6px 32px', fontSize: '0.75rem', color: 'white', width: '250px' }} + style={{ backgroundColor: 'var(--bg-color)', border: '1px solid var(--border-color)', borderRadius: '6px', padding: '6px 12px 6px 32px', fontSize: '0.75rem', color: 'var(--text-primary)', width: '100%', maxWidth: '250px' }} />
@@ -1927,80 +1943,43 @@ const SourceLogModal: React.FC<{ source?: string | null, loading?: boolean, sour } } - // Render to document.body via portal. The InsightCard host uses `.card` - // which sets `backdrop-filter` — and any ancestor with backdrop-filter, + // `Modal` portals to document.body itself, which is what this needed: the + // InsightCard host is a `.card`, and any ancestor with backdrop-filter, // filter, transform or perspective becomes a new containing block for - // `position: fixed`, which previously trapped the modal inside the card - // (it looked half-cut / off-screen). Portaling outside the card fixes it. - const modal = ( -
-
e.stopPropagation()} - style={{ - width: 'min(1100px, 96vw)', - height: 'min(88vh, 800px)', - display: 'flex', - flexDirection: 'column', - padding: 0, - overflow: 'hidden', - backgroundColor: 'var(--card-bg, #0f172a)', - border: '1px solid var(--border-color, #334155)', - borderRadius: '12px', - boxShadow: '0 25px 60px rgba(0,0,0,0.6)', - }} - > -
-
-
Source Log Analyzed by AI
-
- {sourceLabel(sourceFile)} · {timestamp || '-'} -
-
- -
-
- {pretty ? ( - 
-              {pretty}
-
- ) : ( -
- Raw log was not stored when this insight was saved.
- (The "View Source" feature is active for new insights only.) -
- )} -
-
-
+ {pretty} + + ) : ( + + )} + ); - - if (typeof document === 'undefined') return modal; - return createPortal(modal, document.body); }; const InsightSection: React.FC<{ label: string, children: React.ReactNode }> = ({ label, children }) => ( @@ -2152,8 +2131,84 @@ const SoarTab: React.FC<{ data: any, agentName: string, onRefresh: () => void }>
); +/* `window.innerWidth > 768 ? '1fr 1fr' : '1fr'` was here, and it is the one + kind of responsive that is worse than none: the width is read once, while + React renders. Resizing the window changes nothing, because nothing tells + this component to render again - so the layout is decided by whatever the + window happened to be when the tab was opened, and stays there. It looks + responsive to anyone who tests by reloading at each size. + + CSS re-evaluates on every resize by definition. `responsive-grid` is the + class the rest of the console uses. */ +const POSTURE_TONE: Record = { + CRITICAL: 'critical', HIGH: 'high', MEDIUM: 'medium', + LOW: 'low', INFO: 'info', +}; + +const POSTURE_RANK: Record = { + CRITICAL: 0, HIGH: 1, MEDIUM: 2, LOW: 3, INFO: 4, +}; + +/** How this host is configured, as opposed to what happened on it. + * + * These are states, not events: "the Guest account is enabled" is true until + * somebody changes it, not something that occurred at 14:03. The agent + * deduplicates on the finding itself, so a row appears once and stays until + * it is cleared - a cycle that finds nothing new writes nothing. + */ +const PostureTab: React.FC<{ findings: any[] }> = ({ findings }) => { + const rows = [...(findings || [])].sort((a, b) => + (POSTURE_RANK[(a.severity || '').toUpperCase()] ?? 9) + - (POSTURE_RANK[(b.severity || '').toUpperCase()] ?? 9)); + + if (rows.length === 0) { + return ( + + {/* Unlike the telemetry tabs, empty here is a real answer - a host + with nothing misconfigured produces no rows. Saying which is the + whole point; an empty table that might mean "not running" is the + state this table was in for the life of the product. */} + + + ); + } + + return ( + +

+ How this host is configured, not what happened on it. A finding stays + until the setting behind it changes; severity is what an attacker could + do with it rather than how alarming it sounds. +

+ + {rows.map((row, i) => ( + + + + {row.severity || 'unknown'} + + + {row.category || '—'} + {row.finding} + + {row.details || '—'} + + {row.timestamp || '—'} + + ))} + + + ); +}; + const SystemTab: React.FC<{ data: any }> = ({ data }) => ( -
768 ? '1fr 1fr' : '1fr', gap: '32px' }}> +

Hardware Resources

diff --git a/frontend/src/pages/Agents.tsx b/frontend/src/pages/Agents.tsx index 9bfb521..b00a3cd 100644 --- a/frontend/src/pages/Agents.tsx +++ b/frontend/src/pages/Agents.tsx @@ -1,5 +1,5 @@ import React, { useEffect, useMemo, useState } from 'react'; -import { Card } from '../components/ui'; +import { Card, Modal, DialogButton, ErrorState } from '../components/ui'; import { CategoryBars } from '../components/ui/charts'; const chartNote: React.CSSProperties = { @@ -21,8 +21,7 @@ import { RefreshCw, FileDown, ChevronRight, - Trash2, - AlertTriangle + Trash2 } from 'lucide-react'; import { agentService } from '../services/api'; import { saveBlobResponse } from '../utils/downloadBlob'; @@ -265,57 +264,30 @@ const Agents: React.FC = () => {
{pendingDelete && ( -
!deleting && setPendingDelete(null)} - style={{ position: 'fixed', inset: 0, backgroundColor: 'rgba(0,0,0,0.6)', display: 'flex', alignItems: 'center', justifyContent: 'center', zIndex: 1000, padding: '20px' }} + !deleting && setPendingDelete(null)} + width={480} + footer={ + <> + setPendingDelete(null)}>Cancel + + {deleting ? 'Deleting…' : 'Delete permanently'} + + + } > -
e.stopPropagation()} style={{ maxWidth: '480px', width: '100%', padding: '24px' }}> -
- -
-

- Delete {pendingDelete}? -

-

- This drops the agent's telemetry database and removes its - enrolment identity. Every event, alert and AI verdict it - produced is deleted. This cannot be undone. -

-

- A running agent will re-enrol under a new name. Uninstall it - first, or use Self-Destruct from its detail page. -

-
-
- - {deleteError && ( -
- {deleteError} -
- )} - -
- - +

+ A running agent will re-enrol under a new name. Uninstall it first, + or use Self-Destruct from its detail page. +

+ {deleteError && ( +
+
-
-
+ )} + )}
); diff --git a/frontend/src/pages/Assets.tsx b/frontend/src/pages/Assets.tsx index a08edba..f01f4f7 100644 --- a/frontend/src/pages/Assets.tsx +++ b/frontend/src/pages/Assets.tsx @@ -139,7 +139,8 @@ const Assets: React.FC = () => { padding: '12px 16px 12px 48px', borderRadius: '12px', color: 'white', - width: '300px', + width: '100%', + maxWidth: '300px', fontSize: '0.9rem', transition: 'all 0.3s ease', outline: 'none' @@ -155,7 +156,7 @@ const Assets: React.FC = () => {
-
+
{/* Agents Sidebar */}
diff --git a/frontend/src/pages/AuditLogs.tsx b/frontend/src/pages/AuditLogs.tsx index d5e02c9..99b2d88 100644 --- a/frontend/src/pages/AuditLogs.tsx +++ b/frontend/src/pages/AuditLogs.tsx @@ -1,7 +1,7 @@ import React, { useState, useEffect, useMemo } from 'react'; import { User, Activity, Globe, Search, Eye } from 'lucide-react'; import { adminService, agentService } from '../services/api'; -import { Card, Badge } from '../components/ui'; +import { Card, Badge, Modal } from '../components/ui'; /** Platform events carry a severity, and a severity is a claim - so it takes * the semantic colours rather than a series palette. */ @@ -214,7 +214,7 @@ const AuditLogs: React.FC = () => { value={searchTerm} onChange={(e) => setSearchTerm(e.target.value)} onKeyDown={handleSearch} - style={{ backgroundColor: 'var(--bg-color)', border: '1px solid var(--border-color)', borderRadius: '6px', padding: '6px 12px 6px 32px', fontSize: '0.75rem', color: 'var(--text-primary)', width: '300px' }} + style={{ backgroundColor: 'var(--bg-color)', border: '1px solid var(--border-color)', borderRadius: '6px', padding: '6px 12px 6px 32px', fontSize: '0.75rem', color: 'var(--text-primary)', width: '100%', maxWidth: '300px' }} />
@@ -329,59 +329,46 @@ const AuditDetailModal: React.FC<{ log: any, onClose: () => void }> = ({ log, on const known = new Set(['id', 'timestamp', '@timestamp', 'username', 'user_id', 'action', 'resource', 'ip_address', 'details']); const extras = Object.entries(log).filter(([k]) => !known.has(k)); + const row = (label: string, value: unknown, mono = false) => ( + + + {label} + + + {value === '' || value == null + ? — + : String(value)} + + + ); + return ( -
-
e.stopPropagation()} - style={{ - width: 'min(720px, 100%)', maxHeight: '80vh', - display: 'flex', flexDirection: 'column', - backgroundColor: 'var(--card-bg)', border: '1px solid var(--border-color)', borderRadius: '12px', overflow: 'hidden', - }} - > -
-
-
Audit Log Detail
-
{ts}
-
- -
-
- - - {fields.map(([label, value]) => ( - - - - - ))} - {extras.length > 0 && ( - - - - )} - {extras.map(([k, v]) => ( - - - - - ))} - -
{label} - {value === '' || value == null ? — : String(value)} -
Other
{k} - {typeof v === 'object' ? JSON.stringify(v, null, 2) : String(v)} -
-
-
-
+ + + + {fields.map(([label, value]) => row(label, value, label === 'Resource'))} + {extras.length > 0 && ( + + + + )} + {extras.map(([k, v]) => row( + k, typeof v === 'object' ? JSON.stringify(v, null, 2) : v, true))} + +
+ Other +
+ ); }; diff --git a/frontend/src/pages/Dashboard.tsx b/frontend/src/pages/Dashboard.tsx index 5711301..00bb4f8 100644 --- a/frontend/src/pages/Dashboard.tsx +++ b/frontend/src/pages/Dashboard.tsx @@ -375,7 +375,13 @@ const Dashboard: React.FC = () => {

) : ( <> -
+
{ {error && }
diff --git a/frontend/src/pages/Deployment.tsx b/frontend/src/pages/Deployment.tsx index ff2a4eb..c8cbf90 100644 --- a/frontend/src/pages/Deployment.tsx +++ b/frontend/src/pages/Deployment.tsx @@ -246,14 +246,19 @@ const Deployment: React.FC = () => {

-
+ {/* The button was a fourth grid cell with `alignItems: 'end'`, which + lines every item up by its bottom edge — and two of these fields + carry a hint under the input while the third does not, so the + three inputs sat at three different heights and the button at a + fourth. It is an action, not a field; it belongs after them. */} +
setHostnameHint(e.target.value)} /> setNote(e.target.value)} /> - + { onChange={(e) => setTtlHours(parseInt(e.target.value || '24', 10))} /> -
+ {lastToken && (
{ padding: '12px 16px 12px 48px', borderRadius: '12px', color: 'white', - width: '300px', + width: '100%', + maxWidth: '300px', fontSize: '0.9rem', transition: 'all 0.3s ease', outline: 'none' @@ -156,7 +157,7 @@ const FileIntegrity: React.FC = () => {
-
+
{/* Agents Sidebar */}
diff --git a/frontend/src/pages/LoginLogs.tsx b/frontend/src/pages/LoginLogs.tsx index 36ee18b..efee92d 100644 --- a/frontend/src/pages/LoginLogs.tsx +++ b/frontend/src/pages/LoginLogs.tsx @@ -77,7 +77,7 @@ const LoginLogs: React.FC = () => {

Login History

- +
diff --git a/frontend/src/pages/Playbooks.tsx b/frontend/src/pages/Playbooks.tsx index b9ec116..801e34f 100644 --- a/frontend/src/pages/Playbooks.tsx +++ b/frontend/src/pages/Playbooks.tsx @@ -491,12 +491,7 @@ const Playbooks: React.FC = () => {

)} -
+
{playbookList()} {runList()}
diff --git a/frontend/src/pages/SoarHub.tsx b/frontend/src/pages/SoarHub.tsx index e4f33ef..91ad935 100644 --- a/frontend/src/pages/SoarHub.tsx +++ b/frontend/src/pages/SoarHub.tsx @@ -422,7 +422,7 @@ const SoarHub: React.FC = () => { placeholder="Filter events..." value={searchTerm} onChange={(e) => setSearchTerm(e.target.value)} - style={{ backgroundColor: 'var(--bg-color)', border: '1px solid var(--border-color)', borderRadius: '6px', padding: '6px 12px 6px 32px', fontSize: '0.875rem', color: 'white', width: '200px' }} + style={{ backgroundColor: 'var(--bg-color)', border: '1px solid var(--border-color)', borderRadius: '6px', padding: '6px 12px 6px 32px', fontSize: '0.875rem', color: 'white', width: '100%', maxWidth: '200px' }} />
diff --git a/frontend/src/services/api.ts b/frontend/src/services/api.ts index b484166..064a3fa 100644 --- a/frontend/src/services/api.ts +++ b/frontend/src/services/api.ts @@ -196,6 +196,8 @@ export const agentService = { getVulnerabilities: (agent: string) => api.get(`/${agent}/vulnerabilities_report`).then(res => res.data), getPortscans: (agent: string) => api.get(`/${agent}/portscan_result`).then(res => res.data), getCriticalFiles: (agent: string) => api.get(`/${agent}/critical_files`).then(res => res.data), + /** Posture findings: how the host is configured, not what happened. */ + getSecurityAudit: (agent: string) => api.get(`/${agent}/security_audit`).then(res => res.data), getPackages: (agent: string) => api.get(`/${agent}/packages`).then(res => res.data), getDockerContainers: (agent: string) => api.get(`/${agent}/docker_containers`).then(res => res.data), getAgentInfo: (agent: string) => api.get(`/${agent}/agent_info`).then(res => res.data), diff --git a/scanners/vuln.py b/scanners/vuln.py index 3874047..b4e1ff7 100644 --- a/scanners/vuln.py +++ b/scanners/vuln.py @@ -562,7 +562,28 @@ async def scan_agent(agent: str, fernet_key: str | bytes, connect_db_for_agent: if not ecosystem or ecosystem == "ARCH": return {"agent": agent, "ecosystem": ecosystem, "packages": len(pkgs), "hits": 0, "inserted": 0, "skipped_reason": "unsupported_ecosystem"} - query_eco = ecosystem if ecosystem != "Windows" else "NuGet" + if ecosystem == "Windows": + # Skipped, and said so, rather than queried against an ecosystem it + # cannot match. + # + # This used to map Windows to `NuGet`. The package list for a Windows + # agent comes from walking the registry's `Uninstall` keys, so the + # names are display names - "Google Chrome", "Microsoft Visual C++ + # 2015-2022 Redistributable (x64)". NuGet is .NET libraries: + # `Newtonsoft.Json`, `System.Text.Json`. The two name spaces do not + # overlap, and OSV has no ecosystem for "programs installed on + # Windows" at all. + # + # So the query could never match, and every cycle reported + # `new_findings=0` - which on a security console reads as "this host + # has no known vulnerabilities". It is not a clean result; it is no + # result, and the difference is the whole point. A coincidental match + # against a real NuGet package would have been worse than none. + return {"agent": agent, "ecosystem": ecosystem, "packages": len(pkgs), + "hits": 0, "inserted": 0, + "skipped_reason": "windows_inventory_has_no_osv_ecosystem"} + + query_eco = ecosystem triples = _build_triplets(pkgs, ecosystem) if not triples: diff --git a/tests/test_app_shadowed_names.py b/tests/test_app_shadowed_names.py new file mode 100644 index 0000000..89addd1 --- /dev/null +++ b/tests/test_app_shadowed_names.py @@ -0,0 +1,88 @@ +"""In `app.py`, `json` is not the json module. + +Line 12: + + from sanic.response import json, file, HTTPResponse, raw + +so the bare name `json` is Sanic's *response helper*. The standard library is +imported beside it as `pyjson`, and `sanic_json` is a third alias for the same +response function. + +Calling `json.loads(...)` there therefore raises + + 'function' object has no attribute 'loads' + +at request time, and only at request time. It is valid Python, it imports +cleanly, and every source-reading test passes — the WebAuthn registration +route shipped with exactly this and returned 500 on the first click, after a +suite of tests that checked the route existed, was permissioned, read its +challenge from the database and deleted it afterwards. All true; none of them +executed a line of it. + +`file` and `raw` are shadowed the same way, and `file` is the more dangerous of +the two: `file(...)` is a Sanic response and reads like a builtin. +""" +from __future__ import annotations + +import ast +import pathlib + +import pytest + +ROOT = pathlib.Path(__file__).resolve().parent.parent +APP = ROOT / "app.py" + +#: Names `app.py` binds to something other than what a reader assumes, mapped +#: to the attribute access that gives the mistake away. +SHADOWED = { + "json": ("loads", "dumps", "load", "dump", "JSONDecodeError"), + "file": ("read", "write", "open", "close"), + "raw": ("encode", "decode"), +} + + +def _tree() -> ast.Module: + return ast.parse(APP.read_text(encoding="utf-8")) + + +def test_the_shadowing_still_exists(): + """If Sanic's helpers stop being imported bare, this whole file can go. + + Asserted rather than assumed, so the check cannot quietly become a rule + about nothing. + """ + source = APP.read_text(encoding="utf-8") + assert "from sanic.response import json" in source, ( + "`json` is no longer shadowed in app.py — these checks are obsolete" + ) + assert "import json as pyjson" in source, "the real module has no alias" + + +@pytest.mark.parametrize("name", sorted(SHADOWED)) +def test_no_module_attribute_is_read_off_a_shadowed_name(name): + """The check that would have caught the 500 before a person did.""" + offenders = [] + for node in ast.walk(_tree()): + if not isinstance(node, ast.Attribute): + continue + value = node.value + if not isinstance(value, ast.Name) or value.id != name: + continue + if node.attr in SHADOWED[name]: + offenders.append(f"line {node.lineno}: {name}.{node.attr}") + + assert not offenders, ( + f"`{name}` in app.py is Sanic's response helper, not what this call " + f"expects — it raises at request time and nowhere else. " + + "; ".join(offenders) + + (". Use `pyjson`." if name == "json" else ".") + ) + + +def test_the_webauthn_routes_use_the_real_json_module(): + """Named specifically, because this is where it happened. The generic + check above would pass again if somebody reintroduced it with a different + attribute name.""" + source = APP.read_text(encoding="utf-8") + assert "pyjson.loads(lib[" in source + assert "json.loads(lib[" not in source.replace("pyjson.loads(lib[", "") diff --git a/tests/test_auth_wiring.py b/tests/test_auth_wiring.py index 96dcc33..7e0999d 100644 --- a/tests/test_auth_wiring.py +++ b/tests/test_auth_wiring.py @@ -414,3 +414,103 @@ def test_every_guard_names_a_permission_that_exists(): f"these guards name permissions that db/init_userdb.sql never " f"creates, so the routes behind them are unreachable: {invented}" ) + + +# -------------------------------------------------------------------------- +# Two lists that look like the same decision and are not +# -------------------------------------------------------------------------- + +def _route_paths(fn: ast.AST) -> list[str]: + """The URL(s) a handler is mounted at, from its decorators.""" + paths = [] + for dec in getattr(fn, "decorator_list", []): + if not isinstance(dec, ast.Call): + continue + target = dec.func + name = getattr(target, "attr", None) + if name not in ROUTE_DECORATORS: + continue + for arg in dec.args: + if isinstance(arg, ast.Constant) and isinstance(arg.value, str): + paths.append(arg.value) + return paths + + +def test_every_login_ceremony_route_is_reachable_without_a_session(tree): + """A route that authenticates with a pending token must be public. + + The rule is derivable rather than a list to maintain: a handler that looks + up `totp_pending` and never asks for a user id is, by construction, the + second half of a login. There is no session yet — that is the point — so + the session middleware must not reject it before the handler runs. + + `webauthn_login_begin` and `webauthn_login_finish` were added to + `_SESSION_ONLY` in this file and not to `_PUBLIC_EXACT_PATHS` in app.py. + The two read as the same decision and are not: this file's list says "no + permission check is expected here", app.py's says "the middleware must let + it through". So both handlers answered 401 without running, and the + console showed "Authentication required" on the login page — where the + operator is, necessarily, not authenticated yet. + + Every test written for those routes passed. None of them made a request. + + The first version of this check read `_PUBLIC_EXACT_PATHS` and went green + while the routes still 401ed, because that is not the gate either: it is a + path-shaped mirror consulted only when the router cannot resolve a + handler. `authenticate` matches on handler *name* against + `_PUBLIC_HANDLERS`. So the check is on the primary list, and the mirror is + verified separately below. + """ + public = _literal_set(tree, "_PUBLIC_HANDLERS") + + unreachable = sorted( + name for name, (_names, fn) in _route_functions(tree).items() + if "hash_pending" in ast.unparse(fn) + and "current_user_id" not in ast.unparse(fn) + and name not in public + ) + assert not unreachable, ( + "these finish a login with a pending token and are not in " + "_PUBLIC_HANDLERS, so `authenticate` answers 401 before the handler " + f"runs: {unreachable}" + ) + + +def test_the_backstop_mirrors_the_handler_list_for_the_login_ceremony(tree): + """`_PUBLIC_EXACT_PATHS` calls itself a mirror of `_PUBLIC_HANDLERS`. + + A mirror that has drifted is worse than no mirror: it is consulted exactly + when the primary list could not be used, which is the moment nobody is + watching. Checked for the login routes specifically, since the path list is + deliberately narrower than the handler list everywhere else. + """ + paths = _literal_set(tree, "_PUBLIC_EXACT_PATHS") + handlers = _literal_set(tree, "_PUBLIC_HANDLERS") + + missing = [] + for name, (_names, fn) in _route_functions(tree).items(): + if "hash_pending" not in ast.unparse(fn): + continue + if "current_user_id" in ast.unparse(fn): + continue + if name not in handlers: + continue # the check above owns that failure + for path in _route_paths(fn): + if path not in paths: + missing.append(f"{name} at {path}") + + assert not missing, ( + f"public by handler name and absent from the path backstop: {missing}. " + f"If the router ever fails to resolve one of these, the login page " + f"stops working with no way to see why." + ) + + +def test_the_pending_token_scan_matches_something(tree): + """Guard against the check above passing because it found no routes.""" + matched = [name for name, (_n, fn) in _route_functions(tree).items() + if "hash_pending" in ast.unparse(fn) + and "current_user_id" not in ast.unparse(fn)] + assert len(matched) >= 3, ( + f"only {matched} look like login-ceremony routes; the scan is broken" + ) diff --git a/tests/test_console_responsive.py b/tests/test_console_responsive.py new file mode 100644 index 0000000..a67bc1b --- /dev/null +++ b/tests/test_console_responsive.py @@ -0,0 +1,180 @@ +"""The console has to work on a narrow screen, and an inline style cannot. + +The shared kit says this in its own docstring — "responsiveness lives here +rather than in each page; a page that lays itself out cannot be made responsive +twenty times" — and six pages had done it anyway, because a two-column layout +is three words of `gridTemplateColumns` and the breakpoint it needs is not +expressible there at all. + +The result was not a page that degraded. It was two columns at 400px wide, with +the sidebar and the table each too narrow to read and the body scrolling +sideways. + +One of them was worse than a fixed layout: + + gridTemplateColumns: window.innerWidth > 768 ? '1fr 1fr' : '1fr' + +That reads the width once, while React renders. Resizing changes nothing — +nothing tells the component to render again — so the layout is whatever the +window happened to be when the tab was opened. It passes a test done by +reloading at each size, and fails the one done by dragging the window. +""" +from __future__ import annotations + +import pathlib +import re + +import pytest + +ROOT = pathlib.Path(__file__).resolve().parent.parent +PAGES = ROOT / "frontend" / "src" / "pages" +CSS = ROOT / "frontend" / "src" / "index.css" + +PAGE_FILES = sorted(PAGES.glob("*.tsx")) + + +_BLOCK_COMMENT = re.compile(r"/\*.*?\*/", re.S) +_LINE_COMMENT = re.compile(r"(? str: + """The file with its comments removed. + + Every one of these checks is about what the code does, and every fix here + left a comment explaining the thing it replaced — so a scanner that reads + comments finds `window.innerWidth` in the paragraph that says why + `window.innerWidth` is gone, and reports the fix as the bug. + + The `(?` was + # reformatted onto its own line — which is the same "looks fine, + # checks nothing" failure the whole file exists to catch. It was + # caught here by reformatting one. + opening = src.rfind("<", 0, match.start()) + tag = src[opening:opening + 4] if opening != -1 else "" + if tag.startswith(("` has to do the same, or the whole body scrolls sideways and + the sidebar drifts with it.""" + css = _source(CSS) + assert ".table-responsive" in css + + offenders = [] + for path in PAGE_FILES: + src = _source(path) + tables = src.count(" wrappers: + offenders.append(f"{path.name} ({tables} tables, {wrappers} wrappers)") + # AuditLogs' third table is a two-column label/value list inside a dialog + # that scrolls vertically, with `wordBreak: break-all` on the value - it has + # nothing to scroll sideways. + offenders = [o for o in offenders if not o.startswith("AuditLogs.tsx")] + assert not offenders, f"tables with no scroll container: {offenders}" diff --git a/tests/test_frontend_typecheck.py b/tests/test_frontend_typecheck.py new file mode 100644 index 0000000..c0c8a9a --- /dev/null +++ b/tests/test_frontend_typecheck.py @@ -0,0 +1,104 @@ +"""`tsc --noEmit -p tsconfig.json` checks nothing in this project. + +It exits 0 and prints nothing, for any code, however broken — which is the +worst possible behaviour for a verification command, because silence is what +you are looking for. + +The reason is the layout. `frontend/tsconfig.json` is a *solution* file: + + { "files": [], "references": [{"path": "./tsconfig.app.json"}, + {"path": "./tsconfig.node.json"}] } + +`"files": []` means it owns no sources. Pointing `tsc -p` at it type-checks the +empty set and succeeds. The real settings — `noUnusedLocals` among them — live +in the referenced projects, and only `tsc -b` (build mode) follows references. + +`vite build` does not close the gap either: rolldown strips types without +checking them, so it compiles code `tsc` would reject. + +This was not theoretical. Two unused imports left behind by a refactor passed +`tsc --noEmit -p tsconfig.json` and passed `vite build`, and were then caught +by the Docker image build — which runs `npm run build` — after the container +had silently kept serving an old agent binary for a day, because the failing +build had never produced a new image. + +So: the gate is `npm run build`. These tests make that structural rather than +a thing somebody has to remember. +""" +from __future__ import annotations + +import json +import pathlib +import re + +import pytest + +ROOT = pathlib.Path(__file__).resolve().parent.parent +FRONTEND = ROOT / "frontend" + + +def _jsonc(path: pathlib.Path) -> dict: + """tsconfig files are JSON with comments.""" + text = path.read_text(encoding="utf-8") + text = re.sub(r"/\*.*?\*/", "", text, flags=re.S) + text = re.sub(r"(?