From 60f71c0c8a86eb177e765f83c5638a6dd0827244 Mon Sep 17 00:00:00 2001 From: martino <32328813+f3rs3n@users.noreply.github.com> Date: Thu, 17 Sep 2026 18:23:56 +0200 Subject: [PATCH] fix: clarify audit side effects and boot-check scope --- AppImage/messages/en/common.json | 8 +-- AppImage/scripts/audit_checks.py | 7 +- AppImage/scripts/audit_checks_pve.py | 6 +- AppImage/scripts/flask_audit_routes.py | 9 +-- tests/test_audit_safety_wording.cjs | 66 ++++++++++++++++++ tests/test_audit_safety_wording.py | 95 ++++++++++++++++++++++++++ 6 files changed, 177 insertions(+), 14 deletions(-) create mode 100644 tests/test_audit_safety_wording.cjs create mode 100644 tests/test_audit_safety_wording.py diff --git a/AppImage/messages/en/common.json b/AppImage/messages/en/common.json index 51ca198d..1b884451 100644 --- a/AppImage/messages/en/common.json +++ b/AppImage/messages/en/common.json @@ -5081,7 +5081,7 @@ "evidenceObserved": "Observed evidence", "evidenceExcerpt": "Compact view. The complete source evidence remains stored with this assessment.", "annexScope": "Complete source evidence for results that require attention, record an observation or could not be verified.", - "readOnlyScope": "The assessment does not change configuration. Read commands and, when needed, Lynis may generate logs or reports.", + "readOnlyScope": "The assessment inspects host settings and health. It can write reports and logs; boot status checks can temporarily mount EFI system partitions.", "capacity": "Capacity", "used": "Used", "free": "Free", @@ -5182,7 +5182,7 @@ "neverRun": "This host has not been assessed yet.", "lastRun": "Last assessed on {when}", "stale": "{days} days ago", - "readOnlyNotice": "The assessment only reads the host. It makes no changes.", + "readOnlyNotice": "The assessment inspects host settings and health. It can write reports and logs; boot status checks can temporarily mount EFI system partitions.", "unverifiedChecks": "Not taken: {checks}. Each says in its own evidence what it could not read.", "noFindings": "No findings match the current filter.", "affectedCount": "{count} affected", @@ -5403,7 +5403,7 @@ }, "boot_loader": { "title": "Boot loader", - "rationale": "The EFI system partitions proxmox-boot-tool reports and the kernels each one carries. No partition is mounted and no boot is attempted.", + "rationale": "The EFI system partitions proxmox-boot-tool reports and the kernels each one carries. proxmox-boot-tool status can temporarily mount EFI system partitions; no boot is attempted.", "summary": { "synchronised": "The {total} boot partitions carry the same kernels", "attention": "{count} of {total} boot partitions need review", @@ -5891,7 +5891,7 @@ "scope": "Scope of this report", "scopeText": "This document reports the {profile} profile on the node named in the header, at the moment of the run.", "scopeLocal": "It covers this node only. Guests on other nodes and their configuration are outside it.", - "scopeReadOnly": "Every check reads configuration and state that already exists; none modifies the host.", + "scopeReadOnly": "The assessment inspects host settings and health. It can write reports and logs; boot status checks can temporarily mount EFI system partitions.", "scopeMoment": "It describes the state at the time of the run, not a period of time.", "notRead": "Sources that could not be read:", "uplink": "Uplink", diff --git a/AppImage/scripts/audit_checks.py b/AppImage/scripts/audit_checks.py index 2e271915..3deff054 100644 --- a/AppImage/scripts/audit_checks.py +++ b/AppImage/scripts/audit_checks.py @@ -1,8 +1,9 @@ """Check registry and evaluation engine for Audit & Report. A check declares an identifier, an area and the severity its failure -carries, and returns the outcome of one evaluation. Checks never modify -the host: an assessment reads, it does not act. +carries, and returns the outcome of one evaluation. Assessments inspect +host settings and health. They can write reports and logs; boot status +checks can temporarily mount EFI system partitions. Identifiers are ``.`` and are frozen once published. Rewording a title never changes the identifier, because the accepted-risk register @@ -185,7 +186,7 @@ class AuditContext: return self._cache[key] def run(self, cmd: list[str], timeout: int = 10, allowed_codes=(0,)) -> tuple[int, str]: - """Run a read-only command, returning exit code and output.""" + """Run an inspection command, returning exit code and output.""" key = "cmd:" + json.dumps(cmd) self.source(key) if key in self._cache: diff --git a/AppImage/scripts/audit_checks_pve.py b/AppImage/scripts/audit_checks_pve.py index 2fe6b2eb..5ae0ccd4 100644 --- a/AppImage/scripts/audit_checks_pve.py +++ b/AppImage/scripts/audit_checks_pve.py @@ -2785,8 +2785,7 @@ def _update_chain(ctx): inherits the age of that picture. Whether each repository can still be reached is not tested: finding - out means refreshing the indexes, and an assessment that only reads - does not do that. + out means refreshing the indexes, which this check does not do. """ # pkgcache.bin is rebuilt from files already on disk, so its date # says nothing about contacting a repository. These three do, in @@ -3392,7 +3391,8 @@ def _boot_loader(ctx): evidence += ("\nEach partition is an EFI system partition Proxmox keeps in " "step so the host survives losing any one boot disk. The " "kernel each would start is read from the tool's own report; " - "no partition is mounted and no boot is attempted.") + "proxmox-boot-tool status can temporarily mount EFI system partitions; " + "no boot is attempted.") affected = [] for message in problems: diff --git a/AppImage/scripts/flask_audit_routes.py b/AppImage/scripts/flask_audit_routes.py index 777726a4..82e571cd 100644 --- a/AppImage/scripts/flask_audit_routes.py +++ b/AppImage/scripts/flask_audit_routes.py @@ -4,10 +4,11 @@ ProxMenux Audit Routes Flask blueprint for the Audit & Report assessment engine. -An assessment reads the host and records findings; it never modifies -anything. The run endpoint is therefore the only POST that does real -work, and it is deliberately serialised: two concurrent assessments would -compete for the same collectors without producing a better answer. +An assessment inspects host settings and health and records findings. +It can write reports and logs; boot status checks can temporarily mount +EFI system partitions. Assessment runs are deliberately serialised: two +concurrent assessments would compete for the same collectors without +producing a better answer. """ import threading diff --git a/tests/test_audit_safety_wording.cjs b/tests/test_audit_safety_wording.cjs new file mode 100644 index 00000000..6c552650 --- /dev/null +++ b/tests/test_audit_safety_wording.cjs @@ -0,0 +1,66 @@ +// Node 20+: use the same TypeScript compiler harness as the document tests. +// No browser, React rendering, API or host-management imports. +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const { createRequire } = require('node:module'); +const app = path.resolve(__dirname, '../AppImage'); +const appRequire = createRequire(path.join(app, 'package.json')); +const ts = appRequire('typescript'); +const read = rel => fs.readFileSync(path.join(app, rel), 'utf8'); +const cache = new Map(); +function load(file) { + file = path.resolve(file); + if (cache.has(file)) return cache.get(file).exports; + const mod = { exports: {} }; + cache.set(file, mod); + const compiled = ts.transpileModule(fs.readFileSync(file, 'utf8'), { + compilerOptions: { module: ts.ModuleKind.CommonJS, target: ts.ScriptTarget.ES2020 }, + }).outputText; + // Evaluate only local repository modules compiled above, never external input. + new Function('require', 'module', 'exports', compiled)(name => { + if (!name.startsWith('.')) return appRequire(name); + return load(path.resolve(path.dirname(file), name + '.ts')); + }, mod, mod.exports); + return mod.exports; +} +const messages = JSON.parse(read('messages/en/common.json')); +const used = []; +const t = (key, values = {}) => { + used.push(key); + let text = key.split('.').reduce((o, k) => o?.[k], messages); + assert.equal(typeof text, 'string', key); + for (const [k, v] of Object.entries(values)) text = text.replaceAll(`{${k}}`, v); + return text; +}; +const contract = 'The assessment inspects host settings and health. It can write reports and logs; boot status checks can temporarily mount EFI system partitions.'; +global.window = { location: { origin: 'http://localhost' } }; +const { buildAuditDocument } = load(path.join(app, 'lib/audit-document.ts')); +const html = buildAuditDocument({ + t, locale: 'en', profile: 'full', run: null, findings: [], + inventory: { sections: {}, unavailable: {} }, +}); +assert.ok(used.includes('audit.presentation.readOnlyScope'), 'document did not look up its scope contract'); +assert.ok(html.includes(contract), 'real document omitted the safety contract'); + +// Check the JSX-to-catalog binding structurally, not quote style, line breaks, +// class names, or paragraph formatting. This is not a React visibility test. +const report = ts.createSourceFile('audit-report.tsx', read('components/audit-report.tsx'), + ts.ScriptTarget.Latest, true, ts.ScriptKind.TSX); +const noticeKeys = []; +function visit(node) { + if (ts.isJsxExpression(node) && node.expression && ts.isCallExpression(node.expression)) { + const call = node.expression; + const key = call.arguments[0]; + if (ts.isIdentifier(call.expression) && call.expression.text === 't' && + key && ts.isStringLiteral(key) && key.text === 'audit.readOnlyNotice') { + noticeKeys.push(key.text); + } + } + ts.forEachChild(node, visit); +} +visit(report); +assert.ok(noticeKeys.length > 0, 'assessment notice consumer not found'); +for (const key of noticeKeys) assert.equal(t(key), contract); +assert.equal(t('audit.document.scopeReadOnly'), contract); +console.log('PASS: real full document safety contract, JSX notice binding, legacy scope agreement'); diff --git a/tests/test_audit_safety_wording.py b/tests/test_audit_safety_wording.py new file mode 100644 index 00000000..e9044487 --- /dev/null +++ b/tests/test_audit_safety_wording.py @@ -0,0 +1,95 @@ +"""English safety contract at isolated producer seams; no host-module imports. + +Run: python3 -m unittest discover -s tests -p test_audit_safety_wording.py -v +The external boot-tool mount behaviour is documented in the review evidence; +these fixtures exercise ProxMenux's actual command selection and evidence text. +""" +import ast +import json +from pathlib import Path +import re +from types import SimpleNamespace +import unittest + +ROOT = Path(__file__).resolve().parents[1] +SCOPE = ( + "The assessment inspects host settings and health. It can write reports and " + "logs; boot status checks can temporarily mount EFI system partitions." +) +BOOT_EFFECT = ( + "proxmox-boot-tool status can temporarily mount EFI system partitions; " + "no boot is attempted." +) + + +def boot_check(present, output): + """Extract only the check and its two pure helpers, with all I/O replaced.""" + source = ROOT / "AppImage/scripts/audit_checks_pve.py" + tree = ast.parse(source.read_text()) + names = {"_boot_loader", "_version_key", "_unverified"} + nodes = [n for n in tree.body if isinstance(n, ast.FunctionDef) and n.name in names] + assert {n.name for n in nodes} == names + for node in nodes: + node.decorator_list = [] + commands = [] + + def path(value): + assert value == "/etc/kernel/proxmox-boot-uuids", value + return SimpleNamespace(exists=lambda: present) + + def run(argv, **kwargs): + commands.append((argv, kwargs)) + if argv == ["proxmox-boot-tool", "status"]: + return 0, output + if argv == ["uname", "-r"]: + return 0, "6.8.12-1-pve" + raise AssertionError(f"Unmocked command: {argv}") + + env = {"Path": path, "re": re, "json": json, + "CLASS_CONFORMANT": "conformant", "CLASS_WARNING": "warning", + "CLASS_OBSERVATION": "observation", "CLASS_UNVERIFIED": "unverified"} + exec(compile(ast.Module(body=nodes, type_ignores=[]), str(source), "exec"), env) + return env["_boot_loader"](SimpleNamespace(run=run)), commands + + +class SafetyWording(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.audit = json.loads((ROOT / "AppImage/messages/en/common.json").read_text())["audit"] + + def test_boot_evidence_and_rationale_disclose_command_side_effect(self): + for partitions in (1, 2): + with self.subTest(partitions=partitions): + output = "\n".join( + f"ABCD-000{i} is configured with: uefi (versions: 6.8.12-1-pve)" + for i in range(partitions)) + result, commands = boot_check(True, output) + self.assertEqual(commands, [ + (["proxmox-boot-tool", "status"], {"timeout": 25, "allowed_codes": (0, 1)}), + (["uname", "-r"], {}), + ]) + self.assertTrue(result["evidence"].endswith(BOOT_EFFECT), result["evidence"]) + self.assertTrue(self.audit["checks"]["system"]["boot_loader"]["rationale"].endswith(BOOT_EFFECT)) + self.assertEqual(result["summary_params"]["total"], str(partitions)) + + def test_absent_boot_configuration_does_not_invoke_tool(self): + result, commands = boot_check(False, "") + self.assertIsNone(result) + self.assertEqual(commands, []) + + def test_unreadable_boot_status_does_not_claim_success(self): + result, commands = boot_check(True, "E: no configured partitions") + self.assertEqual(result["classification"], "unverified") + self.assertEqual(len(commands), 1) + + def test_all_english_scope_surfaces_share_bounded_contract(self): + # The document currently consumes presentation.readOnlyScope; + # document.scopeReadOnly is retained for existing consumers. + for text in (self.audit["readOnlyNotice"], + self.audit["presentation"]["readOnlyScope"], + self.audit["document"]["scopeReadOnly"]): + self.assertEqual(text, SCOPE) + + +if __name__ == "__main__": + unittest.main()