From 4e23d5478ec0f91db643963daaa5b792a4fec6ce Mon Sep 17 00:00:00 2001 From: xarmian Date: Mon, 2 Nov 2026 06:46:27 +0000 Subject: [PATCH] ci: add scope, result, and source census guards (TASK-4224) Add fail-closed path classification and aggregate-result checks, checked-in web parity policy, AST source censuses, and human-readable PG trace streaming. The strict PG census currently rejects scripts/diagnose-version-ties: its main can reach PostgreSQL, it has no tests, and it is absent from the package list. Keep that list unchanged as instructed. Its classification needs a ruling before this branch can merge; no silent exception is added. --- scripts/ci-pg-census-allow.txt | 5 + scripts/ci-pg-trace.py | 4 + scripts/ci-pg-trace_test.py | 10 + scripts/ci-result.py | 55 +++ scripts/ci-result_test.py | 88 +++++ scripts/ci-scope.py | 103 ++++++ scripts/ci-scope_test.py | 100 ++++++ scripts/ci-web-census-allow.txt | 9 + scripts/ci-web-census/main.go | 546 +++++++++++++++++++++++++++++ scripts/ci-web-census/main_test.go | 123 +++++++ scripts/ci-web-parity.txt | 14 + 11 files changed, 1057 insertions(+) create mode 100644 scripts/ci-pg-census-allow.txt create mode 100644 scripts/ci-result.py create mode 100644 scripts/ci-result_test.py create mode 100644 scripts/ci-scope.py create mode 100644 scripts/ci-scope_test.py create mode 100644 scripts/ci-web-census-allow.txt create mode 100644 scripts/ci-web-census/main.go create mode 100644 scripts/ci-web-census/main_test.go create mode 100644 scripts/ci-web-parity.txt diff --git a/scripts/ci-pg-census-allow.txt b/scripts/ci-pg-census-allow.txt new file mode 100644 index 000000000..2801fc4c6 --- /dev/null +++ b/scripts/ci-pg-census-allow.txt @@ -0,0 +1,5 @@ +# Packagereason. Valid only while storetest usage is NewSQLite/Cleanup. +./internal/decision storetest.NewSQLite only; no PostgreSQL constructor. +./internal/oauth storetest.NewSQLite only; Cleanup cannot create a PostgreSQL template. +# Hash-pinned graph boundary: sql.Open here only retrieves Driver(), then closes; it never dials. +./internal/store.registerGuardedDrivers sha256:ca328ba0209b1304b14957e6e36d0f45712ae7f2731bd5190fb9a05da1224190 Driver registration for SQLite and PG only; no Ping/Conn/query. Any source change requires review. diff --git a/scripts/ci-pg-trace.py b/scripts/ci-pg-trace.py index 3afaebc0f..55d79e676 100755 --- a/scripts/ci-pg-trace.py +++ b/scripts/ci-pg-trace.py @@ -24,6 +24,8 @@ def normalize_package(package, module): def main(): parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("--expect", type=Path, help="expected package list") + parser.add_argument("--show-output", action="store_true", + help="also stream human-readable go test Output events") args = parser.parse_args() connected = set() @@ -35,6 +37,8 @@ def main(): event = json.loads(line) except json.JSONDecodeError: parser.error(f"invalid JSON on stdin line {number}") + if args.show_output and event.get("Action") == "output": + print(event.get("Output", ""), end="", flush=True) if (event.get("Action") == "output" and "PAD_PG_CONNECTED" in event.get("Output", "") and event.get("Package")): diff --git a/scripts/ci-pg-trace_test.py b/scripts/ci-pg-trace_test.py index 5ab168ea1..dbba23016 100644 --- a/scripts/ci-pg-trace_test.py +++ b/scripts/ci-pg-trace_test.py @@ -63,5 +63,15 @@ class TraceTest(unittest.TestCase): self.assertEqual(result.stderr.splitlines(), diagnostics) +class HumanOutputTest(unittest.TestCase): + def test_output_can_be_mirrored_without_losing_trace(self): + row = event(MODULE + '/internal/store', 'ordinary test output\n') + result = subprocess.run([sys.executable, str(SCRIPT), '--show-output'], + input=json.dumps(row) + '\n', text=True, + capture_output=True, check=False) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout, 'ordinary test output\n') + + if __name__ == "__main__": unittest.main() diff --git a/scripts/ci-result.py b/scripts/ci-result.py new file mode 100644 index 000000000..b4185289a --- /dev/null +++ b/scripts/ci-result.py @@ -0,0 +1,55 @@ +#!/usr/bin/env python3 +"""Validate the complete CI needs graph; the sole required check is CI result.""" +import json +import os +import sys + +SCOPED = {'go': 'go', 'go-postgres': 'go', 'go-server-shards': 'go', + 'go-web-parity': 'parity', 'nix': 'nix'} +REQUIRED = {'changes', 'native-smoke', 'web', 'e2e', *SCOPED} + + +def evaluate(needs, event): + errors = [] + if event not in ('push', 'pull_request'): + errors.append(f'unsupported event: {event}') + if not isinstance(needs, dict): + return ['needs must be an object'] + if set(needs) != REQUIRED: + errors.append(f'dependency mismatch: missing={sorted(REQUIRED - set(needs))}, ' + f'extra={sorted(set(needs) - REQUIRED)}') + changes = needs.get('changes', {}) + outputs = changes.get('outputs', {}) if isinstance(changes, dict) else {} + if not isinstance(outputs, dict): + outputs = {} + for key in ('go', 'web', 'nix', 'parity'): + if outputs.get(key) not in ('true', 'false'): + errors.append(f'missing/invalid changes output: {key}') + elif event == 'push' and outputs[key] != 'true': + errors.append(f'push must enable {key}') + for job in sorted(REQUIRED): + record = needs.get(job) + status = record.get('result') if isinstance(record, dict) else None + if status == 'success': + continue + if (status == 'skipped' and event == 'pull_request' and + job in SCOPED and outputs.get(SCOPED[job]) == 'false'): + continue + errors.append(f'{job}: {status!r} is not an authorized result') + return errors + + +def main(): + try: + errors = evaluate(json.loads(os.environ['CI_NEEDS']), os.environ['CI_EVENT_NAME']) + except (KeyError, ValueError) as err: + errors = [str(err)] + for error in errors: + print('::error::' + error, file=sys.stderr) + if not errors: + print('Every required CI dependency passed or was correctly scoped out.') + return bool(errors) + + +if __name__ == '__main__': + sys.exit(main()) diff --git a/scripts/ci-result_test.py b/scripts/ci-result_test.py new file mode 100644 index 000000000..e7adb0af1 --- /dev/null +++ b/scripts/ci-result_test.py @@ -0,0 +1,88 @@ +"""The merge gate must fail closed on skipped or incomplete dependencies.""" +import importlib.util +import json +import os +from pathlib import Path +import subprocess +import sys +import unittest + +SCRIPT = Path(__file__).with_name('ci-result.py') +spec = importlib.util.spec_from_file_location('result', SCRIPT) +result = importlib.util.module_from_spec(spec) +spec.loader.exec_module(result) + + +def needs(): + data = {k: {'result': 'success'} for k in ( + 'changes', 'go', 'go-postgres', 'go-server-shards', 'go-web-parity', + 'nix', 'native-smoke', 'web', 'e2e')} + data['changes']['outputs'] = {'go': 'false', 'web': 'true', 'nix': 'false', 'parity': 'false'} + return data + + +class ResultTest(unittest.TestCase): + def test_expected_skips(self): + data = needs() + for job in ('go', 'go-postgres', 'go-server-shards', 'go-web-parity', 'nix'): + data[job]['result'] = 'skipped' + self.assertEqual(result.evaluate(data, 'pull_request'), []) + + def test_rejects(self): + cases = [] + for job, flag in [('go', 'go'), ('go-postgres', 'go'), ('go-server-shards', 'go'), + ('nix', 'nix'), ('go-web-parity', 'parity')]: + data = needs() + data['changes']['outputs'][flag] = 'true' + data[job]['result'] = 'skipped' + cases.append((f'unexpected {job} skip', data, 'pull_request')) + for job in needs(): + for status in ('failure', 'cancelled', 'unknown', None): + data = needs() + data[job]['result'] = status + cases.append((f'{job} {status}', data, 'pull_request')) + data = needs() + del data[job] + cases.append((f'missing {job}', data, 'pull_request')) + for job in ('native-smoke', 'web', 'e2e', 'changes'): + data = needs() + data[job]['result'] = 'skipped' + cases.append((f'nonlisted skip {job}', data, 'pull_request')) + for job in ('go', 'nix', 'go-web-parity'): + data = needs() + data[job]['result'] = 'skipped' + cases.append(('push skip ' + job, data, 'push')) + for flag in ('go', 'web', 'nix', 'parity'): + data = needs() + del data['changes']['outputs'][flag] + cases.append(('missing output ' + flag, data, 'pull_request')) + cases.append(('unsupported event', needs(), 'schedule')) + for name, data, event in cases: + with self.subTest(name=name): + self.assertTrue(result.evaluate(data, event)) + + def test_push(self): + data = needs() + data['changes']['outputs'] = dict.fromkeys(('go', 'web', 'nix', 'parity'), 'true') + self.assertEqual(result.evaluate(data, 'push'), []) + + def test_cli_contract(self): + for status, want in [('success', 0), ('failure', 1), ('cancelled', 1), ('skipped', 1)]: + data=needs() + data['changes']['outputs']['go']='true' + data['go']['result']=status + proc=subprocess.run([sys.executable,str(SCRIPT)], env={**os.environ, + 'CI_NEEDS':json.dumps(data), 'CI_EVENT_NAME':'pull_request'}, + capture_output=True, text=True) + self.assertEqual(proc.returncode,want,proc.stderr) + + def test_cli_malformed(self): + for raw in ('not json', 'null', '[]', '{}'): + proc = subprocess.run([sys.executable, str(SCRIPT)], env={**os.environ, + 'CI_NEEDS': raw, 'CI_EVENT_NAME': 'pull_request'}, + capture_output=True, text=True) + self.assertNotEqual(proc.returncode, 0, proc.stdout) + + +if __name__ == '__main__': + unittest.main() diff --git a/scripts/ci-scope.py b/scripts/ci-scope.py new file mode 100644 index 000000000..219b53082 --- /dev/null +++ b/scripts/ci-scope.py @@ -0,0 +1,103 @@ +#!/usr/bin/env python3 +"""Fail-closed CI scope. PRs compare exact head against merge-base(base, head). + +GO: everything outside web/. PARITY: any web/ path. WEB: always. +NIX: dependency/packaging policy, deliberately not every derivation source. +Workflows and scripts/ci-* force all jobs. Pushes always run everything. +""" +import fnmatch +import json +import os +from pathlib import Path +import re +import subprocess +import sys + +NIX_FILES = {'go.mod', 'go.sum', 'web/package.json', 'web/package-lock.json', + 'web/npm-shrinkwrap.json', 'web/yarn.lock', 'web/pnpm-lock.yaml', + 'web/bun.lock', 'web/bun.lockb'} + + +def classify(paths): + outputs = dict(go=False, web=True, nix=False, parity=False) + for path in paths: + if path.startswith(('.github/workflows/', 'scripts/ci-')): + return dict.fromkeys(outputs, True) + outputs['go'] |= not path.startswith('web/') + outputs['parity'] |= path.startswith('web/') + outputs['nix'] |= (path.startswith('nix/') or + ('/' not in path and fnmatch.fnmatchcase(path, 'flake.*')) or + path in NIX_FILES) + return outputs + + +def git(*args, cwd=None): + return subprocess.check_output(['git', *args], cwd=cwd) + + +def decode_paths(data): + if not data: + return [] # A successful, genuinely empty diff is valid. + if not data.endswith(b'\0'): + raise ValueError('diff is not NUL terminated') + paths = [os.fsdecode(p) for p in data[:-1].split(b'\0')] + if any(not p or p.startswith('/') or any(s in ('.', '..') for s in p.split('/')) + for p in paths): + raise ValueError('invalid repository-relative diff path') + return paths + + +def pr_paths(base, head, cwd=None): + for sha in (base, head): + if not isinstance(sha, str) or not re.fullmatch(r'[0-9a-fA-F]{40}', sha): + raise ValueError('PR base and head must be exact commit SHAs') + git('cat-file', '-e', sha + '^{commit}', cwd=cwd) + ancestor = git('merge-base', base, head, cwd=cwd).decode('ascii').strip() + if not re.fullmatch(r'[0-9a-fA-F]{40}', ancestor): + raise ValueError('missing or malformed merge base') + return decode_paths(git('diff', '--name-only', '-z', '--no-renames', ancestor, head, cwd=cwd)) + + +def has_local_dependencies(go_mod, package): + # A replacement can introduce extra local inputs. Until classified, keep + # Nix on for every PR; do the same for npm workspaces/file dependencies. + if re.search(r'^\s*replace\b', go_mod, re.MULTILINE) or package.get('workspaces'): + return True + return any(version.startswith(('file:', 'link:', 'workspace:')) + for kind in ('dependencies', 'devDependencies', 'optionalDependencies') + for version in package.get(kind, {}).values()) + + +def for_event(event, payload): + if event == 'push': + return dict.fromkeys(('go', 'web', 'nix', 'parity'), True) + if event != 'pull_request': + raise ValueError('unsupported CI event: ' + event) + try: + pr = payload['pull_request'] + outputs = classify(pr_paths(pr['base']['sha'], pr['head']['sha'])) + root = Path(__file__).resolve().parent.parent + if has_local_dependencies((root / 'go.mod').read_text(), + json.loads((root / 'web/package.json').read_text())): + outputs['nix'] = True + return outputs + except (KeyError, TypeError) as err: + raise ValueError('PR event lacks exact base/head SHAs') from err + + +def main(): + try: + payload = json.loads(Path(os.environ['GITHUB_EVENT_PATH']).read_text()) + outputs = for_event(os.environ['GITHUB_EVENT_NAME'], payload) + text = ''.join(f'{key}={str(value).lower()}\n' for key, value in outputs.items()) + print(text, end='') + with open(os.environ['GITHUB_OUTPUT'], 'a') as output: + output.write(text) + except (OSError, ValueError, KeyError, subprocess.CalledProcessError) as err: + print(f'::error::CI scope failed: {err}', file=sys.stderr) + return 1 + return 0 + + +if __name__ == '__main__': + sys.exit(main()) diff --git a/scripts/ci-scope_test.py b/scripts/ci-scope_test.py new file mode 100644 index 000000000..5e70c28bb --- /dev/null +++ b/scripts/ci-scope_test.py @@ -0,0 +1,100 @@ +"""Path policy and real git diff regression cases (including hostile filenames).""" +import importlib.util +import os +from pathlib import Path +import subprocess +import tempfile +import unittest +from unittest.mock import patch + +SCRIPT = Path(__file__).with_name('ci-scope.py') +spec = importlib.util.spec_from_file_location('scope', SCRIPT) +scope = importlib.util.module_from_spec(spec) +spec.loader.exec_module(scope) + + +class ScopeTest(unittest.TestCase): + def test_paths(self): + for path, want in [ + ('web/static/a', (False, True, False, True)), + ('web/src/a.ts', (False, True, False, True)), + ('internal/store/a.go', (True, True, False, False)), + ('go.mod', (True, True, True, False)), + ('go.sum', (True, True, True, False)), + ('.github/workflows/new.yml', (True, True, True, True)), + ('scripts/ci-web-parity.txt', (True, True, True, True)), + ('scripts/ci-web-census/main.go', (True, True, True, True)), + ('README.md', (True, True, False, False)), + ('nix/package.nix', (True, True, True, False)), + ('flake.lock', (True, True, True, False)), + ('web/package-lock.json', (False, True, True, True)), + ('web/package.json', (False, True, True, True)), + ('web/file with spaces\nand newline', (False, True, False, True)), + ]: + with self.subTest(path=path): + got = scope.classify([path]) + self.assertEqual(tuple(got[k] for k in ('go', 'web', 'nix', 'parity')), want) + + def test_local_dependency_guard(self): + cases = [ + ('module example.invalid/p\n', {}, False), + ('replace example.invalid/dep => ../dep\n', {}, True), + ('replace (\n example.invalid/dep => ./dep\n)\n', {}, True), + ('module example.invalid/p\n', {'workspaces': ['packages/*']}, True), + ('module example.invalid/p\n', {'dependencies': {'dep': 'file:../dep'}}, True), + ('module example.invalid/p\n', {'devDependencies': {'dep': 'workspace:*'}}, True), + ] + for go_mod, package, want in cases: + with self.subTest(go_mod=go_mod, package=package): + self.assertEqual(scope.has_local_dependencies(go_mod, package), want) + + def test_push_and_errors(self): + self.assertTrue(all(scope.for_event('push', {}).values())) + for event, payload in [('schedule', {}), ('pull_request', {}), + ('pull_request', {'pull_request': {'base': {'sha': 'bad'}, 'head': {'sha': 'bad'}}})]: + with self.subTest(event=event, payload=payload), self.assertRaises(ValueError): + scope.for_event(event, payload) + for data in [b'file', b'\0', b'../outside\0', b'/absolute\0', b'a\0\0']: + with self.subTest(data=data), self.assertRaises(ValueError): + scope.decode_paths(data) + self.assertEqual(scope.decode_paths(b''), []) + for returns in ( + [subprocess.CalledProcessError(1, 'cat-file')], + [b'', b'', subprocess.CalledProcessError(1, 'merge-base')], + [b'', b'', b'not-a-sha\n'], + [b'', b'', b'a'*40+b'\n', subprocess.CalledProcessError(1, 'diff')]): + with self.subTest(returns=returns), patch.object(scope, 'git', side_effect=returns): + with self.assertRaises((subprocess.CalledProcessError, ValueError)): + scope.pr_paths('a'*40, 'b'*40) + + def test_merge_base_deletions_and_rename(self): + with tempfile.TemporaryDirectory() as tmp: + def git(*args): + return subprocess.check_output(['git', '-C', tmp, *args]).decode().strip() + def commit(): + git('add', '-A') + git('-c', 'user.name=Test', '-c', 'user.email=test@example.invalid', 'commit', '-qm', 'fixture') + return git('rev-parse', 'HEAD') + git('init', '-q') + root = Path(tmp) + (root / 'web').mkdir() + (root / 'web' / 'old\n name').write_text('original') + ancestor = commit() + (root / 'base-only.go').write_text('base') + base = commit() + # Detached test-fixture history only; never changes the task worktree. + git('checkout', '-q', '--detach', ancestor) + (root / 'web' / 'old\n name').rename(root / 'new name.go') + commit() + (root / 'web' / 'other space').write_text('second commit') + head = commit() + paths = scope.pr_paths(base, head, cwd=tmp) + self.assertEqual(set(paths), {'web/old\n name', 'new name.go', 'web/other space'}) + self.assertTrue(scope.classify(paths)['go']) + self.assertTrue(scope.classify(paths)['parity']) + with self.assertRaises(subprocess.CalledProcessError): + scope.pr_paths('f'*40, head, cwd=tmp) + + +if __name__ == '__main__': + unittest.main() diff --git a/scripts/ci-web-census-allow.txt b/scripts/ci-web-census-allow.txt new file mode 100644 index 000000000..9e6138a0c --- /dev/null +++ b/scripts/ci-web-census-allow.txt @@ -0,0 +1,9 @@ +# Fileexact literal or embed patternreviewed reason. +embed.go all:web/build Production frontend embed; parity builds web before compiling Go. +embed.go web/build-materializer/materializer.js Production materializer embed; parity builds and runs the real bundle. +cmd/pad/cmd_server.go web/build fs.Sub of the reviewed production frontend embed, exercised by parity's web build. +internal/store/items_test.go expected false when only web/skill items exist Diagnostic source labels (web versus skill), not a repository path. +internal/server/plan3555_unguarded_write_test.go %s: cli/content=%v cli/fields=%v web/content=%v, want %v/%v/%v Diagnostic counters by API surface, not repository paths. +internal/server/bug3372_attribution_test.go %s: author=%v created_by=%v source=%v user_id=%v, want Alice/user/web/%s Diagnostic attribution values, not a repository path. +internal/materialize/materialize.go materialize: bundle did not define Materializer (a placeholder? run `node scripts/build-materializer.mjs` in web/) Build-help diagnostic; the real embedded bundle is covered by the full internal/materialize parity entry. +internal/server/bug3372_attribution_test.go create: created_by=%v source=%v, want user/web Diagnostic attribution values ending in the web source label, not a repository directory. diff --git a/scripts/ci-web-census/main.go b/scripts/ci-web-census/main.go new file mode 100644 index 000000000..f2dc26d8a --- /dev/null +++ b/scripts/ci-web-census/main.go @@ -0,0 +1,546 @@ +// ci-web-census checks all tracked Go sources, independent of build tags. +// It conservatively checks references, including diagnostic strings and fixture +// paths. Constants, concatenations and path joins are resolved without executing +// code. A dynamic suffix after a known web root is an error, even in a listed +// package. Calls and function values propagate through local and imported helpers. +package main + +import ( + "crypto/sha256" + "fmt" + "go/ast" + "go/parser" + "go/token" + "os" + "os/exec" + "path" + "regexp" + "sort" + "strconv" + "strings" +) + +const module = "github.com/PerpetualSoftware/pad" +const unknown = "\x00" + +var webPath = regexp.MustCompile(`(^|[^a-zA-Z0-9_-])web/`) + +type source struct { + file, pkg string + tree *ast.File + imports map[string]string +} + +type unit struct { + src *source + name string + node ast.Node + refs map[string]bool + web, pg bool +} + +func packagePath(file string) string { + dir := path.Dir(file) + if dir == "." { + return "." + } + return "./" + dir +} + +func literal(e ast.Expr) string { + if lit, ok := e.(*ast.BasicLit); ok && lit.Kind == token.STRING { + value, _ := strconv.Unquote(lit.Value) + return value + } + return "" +} + +// resolve preserves unknown pieces rather than losing a partially known path. +// Unknown prefixes (a repository or temp root) don't hide a known web suffix; +// unknown suffixes cannot be silently accepted as a resolved reference. +func resolve(e ast.Expr, globals map[string]ast.Expr, seen map[ast.Expr]bool) string { + if e == nil || seen[e] { + return unknown + } + seen[e] = true + defer delete(seen, e) + switch x := e.(type) { + case *ast.BasicLit: + if x.Kind == token.STRING { + return literal(x) + } + case *ast.ParenExpr: + return resolve(x.X, globals, seen) + case *ast.BinaryExpr: + if x.Op == token.ADD { + return resolve(x.X, globals, seen) + resolve(x.Y, globals, seen) + } + case *ast.Ident: + if x.Obj != nil { + switch d := x.Obj.Decl.(type) { + case *ast.ValueSpec: + for i, n := range d.Names { + if n.Name == x.Name && i < len(d.Values) { + return resolve(d.Values[i], globals, seen) + } + } + case *ast.AssignStmt: + for i, n := range d.Lhs { + if n == x || identName(n) == x.Name { + if i < len(d.Rhs) { + return resolve(d.Rhs[i], globals, seen) + } + } + } + } + } + if value, ok := globals[x.Name]; ok { + return resolve(value, globals, seen) + } + case *ast.CallExpr: + if sel, ok := x.Fun.(*ast.SelectorExpr); ok { + base := identName(sel.X) + if imported, ok := globals["#import:"+base]; ok { + switch literal(imported) { + case "path/filepath": + base = "filepath" + case "path": + base = "path" + case "fmt": + base = "fmt" + default: + base = "" + } + } + if base == "fmt" && sel.Sel.Name == "Sprintf" && len(x.Args) > 0 { + format := resolve(x.Args[0], globals, seen) + args := []any{} + for _, arg := range x.Args[1:] { + value := resolve(arg, globals, seen) + if strings.Contains(value, unknown) { + return format + unknown + } + args = append(args, value) + } + return fmt.Sprintf(format, args...) + } + if (base == "filepath" || base == "path") && sel.Sel.Name == "Join" { + parts := []string{} + for _, arg := range x.Args { + parts = append(parts, resolve(arg, globals, seen)) + } + return strings.Join(parts, "/") + } + if (base == "filepath" || base == "path") && (sel.Sel.Name == "Clean" || sel.Sel.Name == "FromSlash" || sel.Sel.Name == "Abs") && len(x.Args) == 1 { + return resolve(x.Args[0], globals, seen) + } + } + } + return unknown +} + +func identName(e ast.Expr) string { + if x, ok := e.(*ast.Ident); ok { + return x.Name + } + return "" +} + +func isWeb(value string) bool { + value = strings.ReplaceAll(value, "\\", "/") + return webPath.MatchString(value) || strings.HasSuffix(value, "/web") +} + +func audit(files map[string]string, parity, allow map[string]string, pgPackages map[string]bool, sqliteAllow map[string]string) []string { + errors := []string{} + sources := []*source{} + globals := map[string]map[string]ast.Expr{} + units := []*unit{} + for file, text := range files { + tree, err := parser.ParseFile(token.NewFileSet(), file, text, parser.ParseComments) + if err != nil { + errors = append(errors, err.Error()) + continue + } + src := &source{file: file, pkg: packagePath(file), tree: tree, imports: map[string]string{}} + sources = append(sources, src) + if globals[src.pkg] == nil { + globals[src.pkg] = map[string]ast.Expr{} + } + for _, imp := range tree.Imports { + value := literal(imp.Path) + alias := path.Base(value) + if imp.Name != nil { + alias = imp.Name.Name + } + src.imports[alias] = value + if alias == "." && (value == "os" || value == "io/fs" || value == "path" || value == "path/filepath") { + errors = append(errors, file+": unresolved filesystem dot import") + } + } + for _, decl := range tree.Decls { + if gen, ok := decl.(*ast.GenDecl); ok && (gen.Tok == token.CONST || gen.Tok == token.VAR) { + for _, spec := range gen.Specs { + v := spec.(*ast.ValueSpec) + for i, n := range v.Names { + if i < len(v.Values) { + globals[src.pkg][n.Name] = v.Values[i] + } + } + } + } + } + } + for _, src := range sources { + for _, decl := range src.tree.Decls { + switch d := decl.(type) { + case *ast.FuncDecl: + units = append(units, &unit{src: src, name: d.Name.Name, node: d, refs: map[string]bool{}}) + case *ast.GenDecl: + if d.Tok == token.IMPORT { + continue + } + for _, spec := range d.Specs { + if typ, ok := spec.(*ast.TypeSpec); ok { + units = append(units, &unit{src: src, name: typ.Name.Name, node: typ.Type, refs: map[string]bool{}}) + } + if v, ok := spec.(*ast.ValueSpec); ok { + // Each binding has its own node in the reference graph, so constants + // and function aliases used from another file cannot disappear. + for i, n := range v.Names { + if i < len(v.Values) { + units = append(units, &unit{src: src, name: n.Name, node: v.Values[i], refs: map[string]bool{}}) + } + } + } + } + } + } + for _, group := range src.tree.Comments { + for _, comment := range group.List { + if !strings.HasPrefix(comment.Text, "//go:embed ") { + continue + } + for _, pattern := range strings.Fields(strings.TrimPrefix(comment.Text, "//go:embed ")) { + pattern = strings.Trim(pattern, "\"`") + if (isWeb(strings.TrimPrefix(pattern, "all:")) || pattern == "web" || pattern == "all:web") && allow[src.file+"\t"+pattern] == "" { + // Embeds cannot be selected with -run. Cover the whole package or + // explicitly review the production embed in the allow-file. + if filter, ok := parity[src.pkg]; !ok || filter != "" { + errors = append(errors, src.file+": uncovered web embed "+pattern) + } + } + } + } + } + } + byID := map[string][]*unit{} + for _, u := range units { + byID[u.src.pkg+"."+u.name] = append(byID[u.src.pkg+"."+u.name], u) + } + for _, u := range units { + bindings := map[string]ast.Expr{} + for name, value := range globals[u.src.pkg] { + bindings[name] = value + } + for alias, imported := range u.src.imports { + bindings["#import:"+alias] = &ast.BasicLit{Kind: token.STRING, Value: strconv.Quote(imported)} + } + usesSQL, pgDriverPiece := false, false + selectorNames := map[*ast.Ident]bool{} + ast.Inspect(u.node, func(n ast.Node) bool { + if n == nil { + return true + } + if expr, ok := n.(ast.Expr); ok { + value := resolve(expr, bindings, map[ast.Expr]bool{}) + if strings.Contains(value, "pgx") || strings.Contains(value, "postgres") { + pgDriverPiece = true + } + if isWeb(value) && allow[u.src.file+"\t"+value] == "" { + u.web = true + // An unknown root before web is safe for scope classification; a + // dynamic component beneath it must be made mechanically resolvable. + start := strings.Index(strings.ReplaceAll(value, "\\", "/"), "web/") + if start >= 0 && strings.Contains(value[start:], unknown) { + errors = append(errors, u.src.file+": unresolved web path in "+u.name) + } + } + } + switch x := n.(type) { + case *ast.BasicLit: + value := literal(x) + if value == "PAD_TEST_"+"POSTGRES_URL" || value == "PAD_TEST_"+"POSTGRES_TOOLS_URL" { + u.pg = true + } + case *ast.Ident: + if selectorNames[x] { + break + } + u.refs[u.src.pkg+"."+x.Name] = true + if dotted := u.src.imports["."]; strings.HasPrefix(dotted, module+"/") { + u.refs["./"+strings.TrimPrefix(dotted, module+"/")+"."+x.Name] = true + } + if x.Name == "NewPostgres" || x.Name == "testStorePostgres" || x.Name == "newPostgresConn" { + u.pg = true + } + case *ast.SelectorExpr: + selectorNames[x.Sel] = true + imported := u.src.imports[identName(x.X)] + if imported == "" { + id := u.src.pkg + "." + x.Sel.Name + for _, target := range byID[id] { + if fn, ok := target.node.(*ast.FuncDecl); ok && fn.Recv != nil { + u.refs[id] = true + } + } + } + if strings.HasPrefix(imported, module+"/") { + u.refs["./"+strings.TrimPrefix(imported, module+"/")+"."+x.Sel.Name] = true + } + if x.Sel.Name == "NewPostgres" || (strings.Contains(imported, "jackc/pgx") && + (x.Sel.Name == "Connect" || x.Sel.Name == "ConnectConfig" || x.Sel.Name == "ConnectWithOptions" || x.Sel.Name == "New" || x.Sel.Name == "NewWithConfig")) { + u.pg = true + } + case *ast.CallExpr: + if sel, ok := x.Fun.(*ast.SelectorExpr); ok { + imported := u.src.imports[identName(sel.X)] + if (imported == "os" || imported == "io/fs") && len(x.Args) > 0 { + if resolve(x.Args[0], bindings, map[ast.Expr]bool{}) == "web" { + u.web = true + } + } + if imported == "os" && (sel.Sel.Name == "Getenv" || sel.Sel.Name == "LookupEnv") && len(x.Args) > 0 { + env := resolve(x.Args[0], bindings, map[ast.Expr]bool{}) + if strings.HasPrefix(env, "PAD_TEST_POSTGRES") { + u.pg = true + } + } + if imported == "database/sql" && (sel.Sel.Name == "Open" || sel.Sel.Name == "OpenDB") { + usesSQL = true + driver := unknown + if len(x.Args) > 0 { + driver = resolve(x.Args[0], bindings, map[ast.Expr]bool{}) + } + if strings.Contains(driver, "pg") || strings.Contains(driver, "postgres") || strings.Contains(driver, unknown) { + u.pg = true + } + } + } + } + return true + }) + if usesSQL && pgDriverPiece { + u.pg = true + } + } + // Reviewed nonconnecting helpers are graph boundaries only while their + // exact source hash matches. Their own PG references still need coverage. + boundaries := map[string]bool{} + for id, review := range sqliteAllow { + if !strings.HasPrefix(review, "sha256:") { + continue + } + for _, u := range byID[id] { + text := files[u.src.file][int(u.node.Pos())-1 : int(u.node.End())-1] + want := strings.Fields(review)[0] + if fmt.Sprintf("sha256:%x", sha256.Sum256([]byte(text))) != want { + errors = append(errors, u.src.file+": reviewed nonconnecting helper changed: "+u.name) + } else { + boundaries[id] = true + } + } + } + // Resolve wrappers and function values to a fixed point, across packages too. + for changed := true; changed; { + changed = false + for _, u := range units { + for ref := range u.refs { + if boundaries[ref] { + continue + } + if sqliteAllow[u.src.pkg] != "" && ref == "./internal/store/storetest.Cleanup" { + continue + } + for _, target := range byID[ref] { + if target.pg && !u.pg { + u.pg = true + changed = true + } + if target.web && !u.web { + u.web = true + changed = true + } + } + } + } + } + // Without go/types, a selector on an imported receiver cannot be resolved + // safely. Importing a package exposing PG-capable methods requires review. + pgMethods := map[string]bool{} + for _, u := range units { + if fn, ok := u.node.(*ast.FuncDecl); ok && fn.Recv != nil && ast.IsExported(u.name) && u.pg { + pgMethods[u.src.pkg] = true + } + } + for _, src := range sources { + for _, imported := range src.imports { + pkg := "./" + strings.TrimPrefix(imported, module+"/") + if pgMethods[pkg] && !pgPackages[src.pkg] { + errors = append(errors, src.file+": unresolved PostgreSQL-capable receiver import "+imported) + } + } + // Imports of PG test helpers require review even if calls are indirect or + // unresolved. SQLite exceptions are deliberately restricted to two symbols. + for alias, imp := range src.imports { + if imp != module+"/internal/store/storetest" || pgPackages[src.pkg] { + continue + } + if sqliteAllow[src.pkg] == "" { + errors = append(errors, src.file+": unclassified PostgreSQL-capable storetest import") + continue + } + ast.Inspect(src.tree, func(n ast.Node) bool { + if sel, ok := n.(*ast.SelectorExpr); ok && identName(sel.X) == alias && sel.Sel.Name != "NewSQLite" && sel.Sel.Name != "Cleanup" { + errors = append(errors, src.file+": SQLite-only exception used "+sel.Sel.Name) + } + return true + }) + } + } + for _, u := range units { + if u.pg && !pgPackages[u.src.pkg] { + errors = append(errors, u.src.file+": unlisted PostgreSQL reference/wrapper "+u.name) + } + if !u.web { + continue + } + filter, covered := parity[u.src.pkg] + if !covered { + errors = append(errors, u.src.file+": uncovered web reference in "+u.name) + continue + } + if filter == "" { + continue + } + re, err := regexp.Compile(filter) + if err != nil { + errors = append(errors, "invalid parity regex: "+err.Error()) + continue + } + if strings.HasPrefix(u.name, "Test") && !re.MatchString(u.name) { + errors = append(errors, u.src.file+": web reader "+u.name+" is outside parity -run") + } else if !strings.HasPrefix(u.name, "Test") { + found := false + for _, caller := range units { + if caller.src.pkg == u.src.pkg && strings.HasPrefix(caller.name, "Test") && caller.web { + found = true + } + } + if !found { + errors = append(errors, u.src.file+": unresolved web helper caller "+u.name) + } + } + } + sort.Strings(errors) + return errors +} + +func readLines(file string) ([]string, error) { + data, err := os.ReadFile(file) + if err != nil { + return nil, err + } + lines := []string{} + for _, line := range strings.Split(string(data), "\n") { + if strings.TrimSpace(line) != "" && !strings.HasPrefix(strings.TrimSpace(line), "#") { + lines = append(lines, line) + } + } + return lines, nil +} + +func run() error { + lines, err := readLines("scripts/ci-web-parity.txt") + if err != nil { + return err + } + parity := map[string]string{} + for _, line := range lines { + fields := strings.Fields(line) + if len(fields) > 2 || !strings.HasPrefix(fields[0], "./") { + return fmt.Errorf("invalid parity entry: %q", line) + } + filter := "" + if len(fields) == 2 { + filter = fields[1] + if _, err := regexp.Compile(filter); err != nil { + return err + } + } + if _, exists := parity[fields[0]]; exists { + return fmt.Errorf("duplicate parity package: %s", fields[0]) + } + parity[fields[0]] = filter + } + allow := map[string]string{} + lines, err = readLines("scripts/ci-web-census-allow.txt") + if err != nil { + return err + } + for _, line := range lines { + fields := strings.SplitN(line, "\t", 3) + if len(fields) != 3 || fields[2] == "" { + return fmt.Errorf("allow entry needs file, exact pattern, reason: %q", line) + } + allow[fields[0]+"\t"+fields[1]] = fields[2] + } + pg := map[string]bool{} + lines, err = readLines("scripts/ci-pg-packages.txt") + if err != nil { + return err + } + for _, line := range lines { + pg[strings.TrimSpace(line)] = true + } + sqlite := map[string]string{} + lines, err = readLines("scripts/ci-pg-census-allow.txt") + if err != nil { + return err + } + for _, line := range lines { + fields := strings.SplitN(line, "\t", 2) + if len(fields) != 2 || fields[1] == "" { + return fmt.Errorf("SQLite allow entry needs package and reason: %q", line) + } + sqlite[fields[0]] = fields[1] + } + output, err := exec.Command("git", "ls-files", "-z", "--", "*.go").Output() + if err != nil { + return err + } + files := map[string]string{} + for _, file := range strings.Split(strings.TrimSuffix(string(output), "\x00"), "\x00") { + data, err := os.ReadFile(file) + if err != nil { + return err + } + files[file] = string(data) + } + if len(files) == 0 { + return fmt.Errorf("no tracked Go sources") + } + errors := audit(files, parity, allow, pg, sqlite) + if len(errors) > 0 { + return fmt.Errorf("source census failed:\n%s", strings.Join(errors, "\n")) + } + fmt.Printf("Web and PostgreSQL census passed (%d tracked Go files).\n", len(files)) + return nil +} + +func main() { + if err := run(); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } +} diff --git a/scripts/ci-web-census/main_test.go b/scripts/ci-web-census/main_test.go new file mode 100644 index 000000000..d03b4e444 --- /dev/null +++ b/scripts/ci-web-census/main_test.go @@ -0,0 +1,123 @@ +package main + +import ( + "crypto/sha256" + "fmt" + "strings" + "testing" +) + +func TestCensus(t *testing.T) { + tests := []struct { + name, source, manifest, allow string + pg bool + want string + }{ + {name: "unlisted web reader", source: `package p; import "os"; func TestRead(t *testing.T) { os.ReadFile("../../web/foo") }`, want: "web"}, + {name: "covered reader", source: `package p; func TestRead(t *testing.T) { read("../../web/foo") }`, manifest: "^TestRead$"}, + {name: "wrong test filter", source: `package p; func TestRead(t *testing.T) { read("../../web/foo") }`, manifest: "^TestOther$", want: "TestRead"}, + {name: "embed", source: "package p\n//go:embed web/build\nvar ui string", want: "web"}, + {name: "reviewed embed", source: "package p\n//go:embed web/build\nvar ui string", allow: "web/build"}, + {name: "unknown formatted path", source: `package p; import "fmt"; func TestRead(t *testing.T) { read(fmt.Sprintf("web/%s", fromEnv())) }`, manifest: ".*", want: "unresolved"}, + {name: "unknown suffix", source: `package p; func TestRead(t *testing.T) { read(filepath.Join("..", "web", fromEnv())) }`, manifest: ".*", want: "unresolved"}, + {name: "filesystem dot import", source: `package p; import . "path/filepath"; func TestRead(t *testing.T) { read(Join("..","web",fromEnv())) }`, manifest: ".*", want: "unresolved"}, + {name: "unrelated dot import", source: `package p; import . "net/http"; var method=MethodGet`}, + {name: "alias join", source: `package p; import fp "path/filepath"; const dir="web"; func TestRead(t *testing.T) { read(fp.Join("..", dir, "foo")) }`, want: "web"}, + {name: "alias unknown suffix", source: `package p; import fp "path/filepath"; const dir="web"; func TestRead(t *testing.T) { read(fp.Join("..", dir, fromEnv())) }`, manifest: ".*", want: "unresolved"}, + {name: "joined pieces", source: `package p; func TestRead(t *testing.T) { read(path.Join("..", "web", "foo")) }`, want: "web"}, + {name: "constant and concatenation", source: `package p; const root = "we" + "b/"; func TestRead(t *testing.T) { read(root + "foo") }`, want: "web"}, + {name: "helper covered", source: `package p; func helper() { read("web/foo") }; func TestRead(t *testing.T) { helper() }`, manifest: "^TestRead$"}, + {name: "helper uncovered caller", source: `package p; func helper() { read("web/foo") }; func TestRead(t *testing.T) { helper() }; func TestOther(t *testing.T) { helper() }`, manifest: "^TestRead$", want: "TestOther"}, + {name: "pg wrapper", source: `package p; func wrapper() { storetest.NewPostgres(t) }; func TestRead(t *testing.T) { wrapper() }`, want: "PostgreSQL"}, + {name: "listed pg", source: `package p; func wrapper() { storetest.NewPostgres(t) }`, pg: true}, + {name: "pg env getter alias", source: `package p; import "os"; var getter=os.Getenv; func TestRead(t *testing.T) { getter("PAD_TEST_POSTGRES_URL") }`, want: "PostgreSQL"}, + {name: "pg env", source: `package p; import "os"; var dsn = os.Getenv("PAD_TEST_POSTGRES_TOOLS_URL")`, want: "PostgreSQL"}, + {name: "PG error type cannot connect", source: `package p; import "github.com/jackc/pgx/v5/pgconn"; func check() { var err *pgconn.PgError; _=err }`}, + {name: "conditional PG driver", source: `package p; import "database/sql"; func wrapper(pg bool) { driver:="sqlite"; if pg { driver="pgx" }; sql.Open(driver,dsn) }`, want: "PostgreSQL"}, + {name: "pg driver", source: `package p; import "database/sql"; func wrapper() { sql.Open("pgx", dsn) }`, want: "PostgreSQL"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + parity := map[string]string{} + if tt.manifest != "" { + parity["./probe"] = tt.manifest + } + allow := map[string]string{} + if tt.allow != "" { + allow["probe/reader_test.go\t"+tt.allow] = "reviewed fixture" + } + pg := map[string]bool{} + if tt.pg { + pg["./probe"] = true + } + errs := audit(map[string]string{"probe/reader_test.go": tt.source}, parity, allow, pg, nil) + got := strings.Join(errs, "\n") + if tt.want == "" && got != "" { + t.Fatal(got) + } + if tt.want != "" && !strings.Contains(got, tt.want) { + t.Fatalf("wanted %q; got %q", tt.want, got) + } + }) + } +} + +func TestPGTransitiveImports(t *testing.T) { + files := map[string]string{ + "helper/helper.go": `package helper; func Open() { storetest.NewPostgres(t) }`, + "other/use_test.go": `package other; import h "github.com/PerpetualSoftware/pad/helper"; func TestUse(t *testing.T) { h.Open() }`, + } + errs := audit(files, nil, nil, map[string]bool{"./helper": true}, nil) + if !strings.Contains(strings.Join(errs, "\n"), "other") { + t.Fatalf("unlisted indirect PG user passed: %v", errs) + } +} + +func TestSQLiteException(t *testing.T) { + for _, constructor := range []string{"NewSQLite", "NewPostgres"} { + files := map[string]string{"internal/decision/use_test.go": `package decision; import "github.com/PerpetualSoftware/pad/internal/store/storetest"; func TestUse(t *testing.T) { storetest.` + constructor + `(t) }`} + errs := audit(files, nil, nil, map[string]bool{"./internal/store/storetest": true}, map[string]string{"./internal/decision": "NewSQLite only"}) + if (len(errs) == 0) != (constructor == "NewSQLite") { + t.Fatalf("%s: %v", constructor, errs) + } + } +} + +func TestReviewedNonconnectingBoundary(t *testing.T) { + body := `func Register() { sql.Open("pgx", "") }` + files := map[string]string{ + "helper/helper.go": `package helper; import "database/sql"; ` + body + `; func SQLite() { Register() }`, + "other/use_test.go": `package other; import "github.com/PerpetualSoftware/pad/helper"; func TestUse(t *testing.T) { helper.SQLite() }`, + } + review := map[string]string{"./helper.Register": fmt.Sprintf("sha256:%x registration only", sha256.Sum256([]byte(body)))} + pg := map[string]bool{"./helper": true} + if errs := audit(files, nil, nil, pg, review); len(errs) != 0 { + t.Fatal(errs) + } + files["helper/helper.go"] = strings.Replace(files["helper/helper.go"], `sql.Open("pgx", "")`, `sql.Open("pgx", ""); db.Ping()`, 1) + if errs := audit(files, nil, nil, pg, review); len(errs) == 0 { + t.Fatal("changed reviewed helper passed") + } +} + +func TestPGMethodWrapper(t *testing.T) { + files := map[string]string{ + "helper/helper.go": `package helper; type Helper struct{}; func (h Helper) Open() { storetest.NewPostgres(t) }`, + "other/use_test.go": `package other; import h "github.com/PerpetualSoftware/pad/helper"; func TestUse(t *testing.T) { x:=h.Helper{}; x.Open() }`, + } + errs := audit(files, nil, nil, map[string]bool{"./helper": true}, nil) + if !strings.Contains(strings.Join(errs, "\n"), "other") { + t.Fatalf("unlisted PG method user passed: %v", errs) + } +} + +func TestTestingCleanupIsNotStoretestCleanup(t *testing.T) { + files := map[string]string{ + "internal/store/storetest/helper.go": `package storetest; import "os"; func NewSQLite(t *testing.T) { t.Cleanup(func(){}) }; func Cleanup() { os.Getenv("PAD_TEST_POSTGRES_URL") }`, + "internal/decision/use_test.go": `package decision; import "github.com/PerpetualSoftware/pad/internal/store/storetest"; func TestUse(t *testing.T) { storetest.NewSQLite(t) }`, + } + errs := audit(files, nil, nil, map[string]bool{"./internal/store/storetest": true}, map[string]string{"./internal/decision": "NewSQLite only"}) + if len(errs) != 0 { + t.Fatal(errs) + } +} diff --git a/scripts/ci-web-parity.txt b/scripts/ci-web-parity.txt new file mode 100644 index 000000000..7e3c43a78 --- /dev/null +++ b/scripts/ci-web-parity.txt @@ -0,0 +1,14 @@ +# One package per line; optional second field is the exact go test -run regex. +# The census checks each web-referencing test and every caller of its helpers. +./cmd/pad ^TestPushRefusalCodesMatchTheWebAllowList$ +./cmd/depages +./internal/attachments ^TestMIMEFamilies(CoverAllowlist|FixtureHasNoStrays|KeysAreNormalized)$ +./internal/cli ^Test(FormatChangesForDisplayMatchesTheJSFixture|AgentNameCopyMatchesResolverOrder|DetectProjectCapturesMakeAndNestedWebSignals|BuildWorkspaceContext)$ +./internal/collab ^TestSchemaVersionMatchesWebClient$ +./internal/links ^TestMarkdownCitationsNameLiveSymbols$ +./internal/models ^Test(TerminalListsMatchWeb|NegativeTerminalsMatchTheWebCopy|TitleLimit.*)$ +./internal/server ^Test(CopyPreflightDropReasonsAreRenderedByTheDialog|SharePageDoesNotImportCollab|NotificationEnrichment_PathMatchesTheWebRouteTree|DerivedSSENamesMatchTheLegacyWireVocabulary)$ +./internal/server/render ^Test(ImgPaintableMimes_MatchesTypeScript|RenderAttachment_ParityWithTypeScript)$ +./internal/materialize +./internal/buildtools ^TestInstallRefresh_WebStampCheckUsesTheCallersCheckout$ +./scripts/ci-web-census