diff --git a/.github/workflows/selftest.yml b/.github/workflows/selftest.yml index 2928aee..5c74204 100644 --- a/.github/workflows/selftest.yml +++ b/.github/workflows/selftest.yml @@ -8,6 +8,7 @@ on: branches: [master] pull_request: +# This workflow only reads the repo and runs tests; no write access needed. permissions: contents: read @@ -48,6 +49,7 @@ jobs: tests/test_gen_sbom.py \ tests/test_sbom.py \ tests/test_sbom_identity.py \ + tests/test_low_findings.py \ tests/test_iar_sbom.py \ tests/test_wolfglass_sync.py @@ -63,6 +65,9 @@ jobs: - name: Run SBOM identity tests run: python -m unittest tests/test_sbom_identity.py + - name: Run low-findings tests + run: python -m unittest tests/test_low_findings.py + - name: Run IAR frontend tests run: python -m unittest tests/test_iar_sbom.py diff --git a/share/frontends/compdb_sbom.py b/share/frontends/compdb_sbom.py index 7727d79..4b93abc 100755 --- a/share/frontends/compdb_sbom.py +++ b/share/frontends/compdb_sbom.py @@ -48,19 +48,40 @@ def entry_tokens(entry): return [] +def macro_name(d): + """Macro name of a -D/-U operand: the text before the first '=' or '(', + so a function-like 'F(a)=a' is keyed as 'F'.""" + return re.split(r'[=(]', d, 1)[0] + + def extract_defines(tokens): - defs = [] + """Net -D macros for one compile command, honouring a later -U that cancels + an earlier -D of the same name (the compiler applies them in order, so a + macro undefined on the command line was not compiled in).""" + active = {} # macro name -> full 'NAME' or 'NAME=VALUE' token; last -D wins i = 0 while i < len(tokens): t = tokens[i] if t == '-D' and i + 1 < len(tokens): - defs.append(tokens[i + 1]) + d = tokens[i + 1] + active[macro_name(d)] = d i += 2 continue if t.startswith('-D'): - defs.append(t[2:]) + d = t[2:] + active[macro_name(d)] = d + i += 1 + continue + if t == '-U' and i + 1 < len(tokens): + active.pop(macro_name(tokens[i + 1]), None) + i += 2 + continue + if t.startswith('-U'): + active.pop(macro_name(t[2:]), None) + i += 1 + continue i += 1 - return defs + return list(active.values()) def main(): @@ -108,6 +129,11 @@ def main(): inc = [re.compile(p) for p in args.include] exc = [re.compile(p) for p in args.exclude] + # When --root is given, only hash sources inside it; a compile_commands.json + # is untrusted input, so a crafted entry naming an absolute path outside the + # product must not pull an unrelated file (and its path) into the SBOM. + root_abs = os.path.realpath(args.root) if args.root else None + srcs, defines, seen = [], set(), set() for entry in db: fpath = entry.get('file') @@ -119,6 +145,12 @@ def main(): fpath = os.path.normpath(fpath) if not fpath.lower().endswith(SRC_EXTS): continue + if root_abs: + real = os.path.realpath(fpath) + if os.path.commonpath([real, root_abs]) != root_abs: + sys.stderr.write( + f"WARNING: skipping source outside --root: {fpath}\n") + continue if inc and not any(r.search(fpath) for r in inc): continue if exc and any(r.search(fpath) for r in exc): diff --git a/share/gen-sbom b/share/gen-sbom index 0f80435..90a06b9 100755 --- a/share/gen-sbom +++ b/share/gen-sbom @@ -2,13 +2,16 @@ """Generate CycloneDX 1.6 and SPDX 2.3 SBOMs for wolfssl.""" import argparse +import errno import hashlib import io import json import os import re +import stat import subprocess import sys +import tempfile import uuid from datetime import datetime, timezone @@ -344,6 +347,48 @@ def build_timestamp(): return dt, dt.strftime('%Y-%m-%dT%H:%M:%SZ') +def write_json_atomic(obj, path): + """Write `obj` as JSON to `path` atomically via a sibling temp file and + os.replace, so a crash or ENOSPC mid-write cannot leave a truncated + document that a later CI step would pick up as valid.""" + try: + st = os.lstat(path) + except FileNotFoundError: + st = None + # Only a new path or a plain regular file gets the temp+replace dance. A + # symlink (including /dev/stdout), device or fifo is written through + # directly so the link target is updated rather than the link replaced. + if st is not None and not stat.S_ISREG(st.st_mode): + with open(path, 'w') as f: + json.dump(obj, f, indent=2) + f.write('\n') + return + # os.replace only needs a writable directory; keep refusing a read-only + # output the way a plain open(path, 'w') does. + if st is not None and not os.access(path, os.W_OK): + raise PermissionError(errno.EACCES, os.strerror(errno.EACCES), path) + if st is not None: + mode = stat.S_IMODE(st.st_mode) + else: + umask = os.umask(0) + os.umask(umask) + mode = 0o666 & ~umask + fd, tmp = tempfile.mkstemp(dir=os.path.dirname(path) or '.', + prefix=os.path.basename(path) + '.') + try: + with os.fdopen(fd, 'w') as f: + json.dump(obj, f, indent=2) + f.write('\n') + os.chmod(tmp, mode) + os.replace(tmp, path) + except BaseException: + try: + os.remove(tmp) + except OSError: + pass + raise + + # Known metadata for optional external dependencies. Version is detected # at runtime via pkg-config; falls back to None. Each entry must describe # the *linked artefact* (so vulnerability scanners like OSV / Grype / Trivy @@ -1046,11 +1091,17 @@ def _collect_srcs(srcs_args, srcs_file): continue paths.append(stripped) + # Dedup on the normalized absolute path, not the raw string, so the same + # path spelled two ways (foo.c and ./foo.c) collapses to one entry instead + # of tripping srcs_merkle_hash's duplicate-basename guard. Symlinks are + # not resolved: an alias with its own basename stays a distinct entry. + # The first spelling is kept for opening/hashing. seen = set() deduped = [] for p in paths: - if p not in seen: - seen.add(p) + key = os.path.normpath(os.path.abspath(p)) + if key not in seen: + seen.add(key) deduped.append(p) if not deduped: @@ -1939,12 +1990,8 @@ def main(): ) try: - with open(args.cdx_out, 'w') as f: - json.dump(cdx, f, indent=2) - f.write('\n') - with open(args.spdx_out, 'w') as f: - json.dump(spdx, f, indent=2) - f.write('\n') + write_json_atomic(cdx, args.cdx_out) + write_json_atomic(spdx, args.spdx_out) except OSError as e: sys.exit(f"ERROR: cannot write SBOM output: {e}") diff --git a/share/sbom-driver.py b/share/sbom-driver.py index 20d8137..e128a6b 100755 --- a/share/sbom-driver.py +++ b/share/sbom-driver.py @@ -86,6 +86,11 @@ def scrub_defines(text): def read_version(version_file, version_macro): """Read a version string from a header, e.g. LIBWOLFBOOT_VERSION_STRING.""" + # An empty macro would make re.escape('') a zero-width prefix, so the pattern + # degrades to the first quoted string in the header (a copyright year, an + # unrelated define) -- return "" rather than a silently wrong version. + if not version_macro: + return "" if not version_file or not os.path.isfile(version_file): return "" pat = re.compile(re.escape(version_macro) + r'\s*"([^"]*)"') diff --git a/share/validate_sbom.py b/share/validate_sbom.py index 6d40df7..fc9db9f 100755 --- a/share/validate_sbom.py +++ b/share/validate_sbom.py @@ -143,10 +143,14 @@ def main(argv): try: with open(path) as f: d = json.load(f) - except FileNotFoundError: - fail(path, "file not found") + except OSError as e: + fail(path, f"cannot read file: {e}") except json.JSONDecodeError as e: fail(path, f"invalid JSON: {e}") + except UnicodeDecodeError as e: + fail(path, f"not UTF-8 text: {e}") + if not isinstance(d, dict): + fail(path, "top-level JSON is not an object") if "bomFormat" in d or path.endswith(".cdx.json"): validate_cyclonedx(path, d, args.name_prefix, args.min_properties, args.require_dep_version) diff --git a/tests/test_low_findings.py b/tests/test_low_findings.py new file mode 100644 index 0000000..2bb4b79 --- /dev/null +++ b/tests/test_low_findings.py @@ -0,0 +1,205 @@ +#!/usr/bin/env python3 +"""Regression tests for the low-severity review-findings bundle (gpex.20).""" + +import json +import os +import stat +import subprocess +import sys +import tempfile +import unittest +from importlib.machinery import SourceFileLoader + +REPO = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +SHARE = os.path.join(REPO, "share") +GEN_SBOM = os.path.join(SHARE, "gen-sbom") +VALIDATE = os.path.join(SHARE, "validate_sbom.py") + +gs = SourceFileLoader("gen_sbom", GEN_SBOM).load_module() +drv = SourceFileLoader("sbom_driver", os.path.join(SHARE, "sbom-driver.py") + ).load_module() +compdb = SourceFileLoader( + "compdb_sbom", os.path.join(SHARE, "frontends", "compdb_sbom.py") +).load_module() + + +class TestAtomicWrite(unittest.TestCase): + def test_writes_content_and_leaves_no_temp(self): + with tempfile.TemporaryDirectory() as d: + p = os.path.join(d, "out.json") + gs.write_json_atomic({"a": 1}, p) + with open(p) as f: + self.assertEqual(json.load(f), {"a": 1}) + self.assertFalse(os.path.exists(p + ".tmp")) + + def test_dev_stdout_redirected_to_file(self): + # `--cdx-out /dev/stdout > out.json`: /dev/stdout is a symlink to the + # redirected file and must be written through, not replaced. + code = ("from importlib.machinery import SourceFileLoader as L; " + "L('g', %r).load_module().write_json_atomic({'a': 1}, " + "'/dev/stdout')" % GEN_SBOM) + with tempfile.TemporaryDirectory() as d: + out = os.path.join(d, "out.json") + with open(out, "w") as f: + r = subprocess.run([sys.executable, "-c", code], stdout=f, + stderr=subprocess.PIPE, text=True) + self.assertEqual(r.returncode, 0, r.stderr) + with open(out) as f: + self.assertEqual(json.load(f), {"a": 1}) + + def test_symlinked_output_is_followed(self): + with tempfile.TemporaryDirectory() as d: + target = os.path.join(d, "target.json") + link = os.path.join(d, "link.json") + with open(target, "w") as f: + f.write("old") + os.symlink(target, link) + gs.write_json_atomic({"a": 1}, link) + self.assertTrue(os.path.islink(link)) + with open(target) as f: + self.assertEqual(json.load(f), {"a": 1}) + + def test_existing_mode_preserved(self): + with tempfile.TemporaryDirectory() as d: + p = os.path.join(d, "out.json") + open(p, "w").close() + os.chmod(p, 0o640) + gs.write_json_atomic({"a": 1}, p) + self.assertEqual(stat.S_IMODE(os.stat(p).st_mode), 0o640) + + @unittest.skipIf(os.geteuid() == 0, "root bypasses file permissions") + def test_read_only_output_refused(self): + with tempfile.TemporaryDirectory() as d: + p = os.path.join(d, "out.json") + with open(p, "w") as f: + f.write("keep") + os.chmod(p, 0o444) + with self.assertRaises(PermissionError): + gs.write_json_atomic({"a": 1}, p) + with open(p) as f: + self.assertEqual(f.read(), "keep") + self.assertEqual(os.listdir(d), ["out.json"]) + + def test_fifo_written_directly(self): + with tempfile.TemporaryDirectory() as d: + p = os.path.join(d, "out.fifo") + os.mkfifo(p) + reader = subprocess.Popen(["cat", p], stdout=subprocess.PIPE) + gs.write_json_atomic({"a": 1}, p) + out, _ = reader.communicate(timeout=10) + self.assertEqual(json.loads(out), {"a": 1}) + self.assertTrue(stat.S_ISFIFO(os.lstat(p).st_mode)) + + +class TestSrcsRealpathDedup(unittest.TestCase): + def test_same_file_two_spellings_collapses(self): + with tempfile.TemporaryDirectory() as d: + f = os.path.join(d, "foo.c") + open(f, "w").close() + got = gs._collect_srcs([f, os.path.join(d, ".", "foo.c")], None) + self.assertEqual(len(got), 1, got) + + def test_symlink_alias_stays_distinct(self): + with tempfile.TemporaryDirectory() as d: + f = os.path.join(d, "foo.c") + alias = os.path.join(d, "bar.c") + open(f, "w").close() + os.symlink(f, alias) + got = gs._collect_srcs([f, alias], None) + self.assertEqual(got, [f, alias]) + + +class TestValidateSbomRobustness(unittest.TestCase): + def _run(self, content_or_none, name="x.cdx.json"): + with tempfile.TemporaryDirectory() as d: + p = os.path.join(d, name) + if content_or_none is not None: + with open(p, "w") as f: + f.write(content_or_none) + return subprocess.run([sys.executable, VALIDATE, p], + capture_output=True, text=True) + + def test_non_dict_json_rejected_cleanly(self): + r = self._run("[1, 2, 3]") + self.assertNotEqual(r.returncode, 0) + self.assertIn("not an object", r.stderr) + self.assertNotIn("Traceback", r.stderr) + + def test_missing_file_no_traceback(self): + r = self._run(None) # file never created + self.assertNotEqual(r.returncode, 0) + self.assertNotIn("Traceback", r.stderr) + + def test_non_utf8_rejected_cleanly(self): + with tempfile.TemporaryDirectory() as d: + p = os.path.join(d, "x.cdx.json") + with open(p, "wb") as f: + f.write(b'{"bomFormat": "\xff\xfe"}') + r = subprocess.run([sys.executable, VALIDATE, p], + capture_output=True, text=True, + env=dict(os.environ, PYTHONUTF8="1")) + self.assertNotEqual(r.returncode, 0) + self.assertIn("FAIL", r.stderr) + self.assertNotIn("Traceback", r.stderr) + + def test_directory_input_rejected_cleanly(self): + with tempfile.TemporaryDirectory() as d: + r = subprocess.run([sys.executable, VALIDATE, d], + capture_output=True, text=True) + self.assertNotEqual(r.returncode, 0) + self.assertIn("FAIL", r.stderr) + self.assertNotIn("Traceback", r.stderr) + + +class TestScrubWindowsPath(unittest.TestCase): + def test_windows_abs_path_redacted(self): + out = drv.scrub_defines('#define SDK "C:\\Users\\me\\sdk"\n') + self.assertNotIn("Users", out) + self.assertIn("SDK", out) + + +class TestReadVersionEmptyMacro(unittest.TestCase): + def test_empty_macro_returns_empty(self): + with tempfile.TemporaryDirectory() as d: + h = os.path.join(d, "v.h") + with open(h, "w") as f: + f.write('#define COPYRIGHT "2026 wolfSSL"\n') + self.assertEqual(drv.read_version(h, ""), "") + + +class TestCompdbUndefine(unittest.TestCase): + def test_later_undef_cancels_define(self): + got = compdb.extract_defines(["-DFOO", "-DBAR=1", "-UFOO"]) + self.assertIn("BAR=1", got) + self.assertNotIn("FOO", got) + + def test_separate_arg_forms(self): + got = compdb.extract_defines(["-D", "A=1", "-U", "A"]) + self.assertNotIn("A=1", got) + + def test_function_like_macro_undef(self): + got = compdb.extract_defines(["-DF(a)=a", "-DG(x)", "-UF", "-UG"]) + self.assertEqual(got, []) + + +class TestCompdbRootSlash(unittest.TestCase): + def test_root_slash_keeps_sources(self): + with tempfile.TemporaryDirectory() as d: + src = os.path.join(d, "a.c") + open(src, "w").close() + db = os.path.join(d, "compile_commands.json") + with open(db, "w") as f: + json.dump([{"directory": d, "file": src, + "arguments": ["cc", "-c", src]}], f) + r = subprocess.run( + [sys.executable, + os.path.join(SHARE, "frontends", "compdb_sbom.py"), db, + "--name", "t", "--root", "/", "--print-only"], + capture_output=True, text=True) + self.assertEqual(r.returncode, 0, r.stderr) + self.assertIn("# 1 sources", r.stdout) + self.assertIn(src, r.stdout) + + +if __name__ == "__main__": + unittest.main(verbosity=2)