Skip to content

Commit 7791b99

Browse files
security(hypatia): triage 42 scanner findings — critical 9→0 (#320)
## Hypatia scanner triage — `critical 9 → 0` Reproduces and triages the full **Hypatia 2.0** neurosymbolic scan of `ephapax` (run locally with no `GITHUB_TOKEN`, default severity `medium`) and drives **critical findings to zero**, with every disposition decided from the actual source and the four-layer proof fence respected throughout. > **Ground truth note:** the local scan finds **42** findings > (**9 critical / 7 high / 26 medium**) — more than the 37/5/7/25 in the original > handoff (the prior session scanned a different checkout). This PR triages the > real 42. Branch is based on `origin/main` (the handoff's > `claude/compassionate-mccarthy-xvwd6r` exists nowhere; only `main` is on origin). ### Result | | critical | high | medium | total | |---|---|---|---|---| | **Before** | 9 | 7 | 26 | 42 | | **After** | **0** | 3 | 23 | 26 | `cargo check` (query/parser/wasm/interp/runtime) passes. `Semantics.v` is **untouched** (fence respected); the only `formal/*.v` edits are documentation comments. ### Criticals (9 → 0) | Cluster | Finding | Disposition | What & why | |---|---|---|---| | **C1** | `Semantics.v` legacy `preservation` admit | `.hypatia-ignore` | Theorem is **provably FALSE** (`Counterexample.v`, 5 `Qed` lemmas); the `Admitted.` is deliberate + owner-fenced (CLAUDE.md 2026-05-27). Never closed/edited — path-exempted only. | | **C2** | `Semantics_L1.v` admits (3 `Admitted` + 14 `admit`) | in-file `hypatia: allow` | Tracked v2 proof-debt (`PROOF-NEEDS.md`): `region_liveness` (false-as-stated), `step_pop` (open research), `preservation_l1` (capstone). Documented, not closed. | | **C3** | `Counterexample_L2_nested.v` admit/coq_admit | in-file `hypatia: allow` | **False positive** — scanner substring-matched *prose* ("No new `Axiom` or `Admitted` markers"); file is fully `Qed`-closed. | | **C4** | `parser/lib.rs` `unwrap_or(0)` ×2 | file-level directive + inline notes | Grammar `integer = ASCII_DIGIT+` → `parse()` fails only on i64 overflow; fn returns `Literal` (no `Result`). Deliberate lossy fallback + TODO. (Downgraded from a `?`-refactor that would have broken 2 uncovered call sites.) | | **C5** | `wasm/lib.rs` `unwrap_or(0)` | file-level directive | `0` is a benign sentinel (`None` ⇒ non-data scrutinee), routed correctly by the `total >= 2` check. | | **C6** | `tools/linter.rs` `unwrap_or(0)` ×8 | file-level directive | `line.find(&pat).unwrap_or(0)+1` = 1-based diagnostic column; `0`⇒column 1 is benign. Tooling crate. | | **C7** | `lib/linter.rs` `unwrap_or(0)` ×2 | `.hypatia-ignore` | Stale 178-line duplicate of the live 920-line `tools/linter.rs` (not a workspace member). Same benign column idiom; flagged to owner for removal. | ### Highs & mediums — real fixes | Cluster | Fix | |---|---| | **H6** | `query/lib.rs`: removed 3 `.get_mut(id).unwrap()` via a behavior-preserving `get_mut` refactor. | | **H1** | `instant-sync.yml`: step-level `if: secrets.FARM_DISPATCH_TOKEN != ''` presence gate. | | **H2** | `scorecard.yml`: job-level `security-events: write` + `id-token: write` + `contents: read` (the reusable's required scopes; previously capped by the workflow-level `contents: read`). | | **M1** | `status-gate.yml` (both jobs) + `push-email-notify.yml`: `timeout-minutes`. | | **M2** | `codeql.yml`: added the `actions` language to the matrix. | | **H3/H7/M4** | `runtime` `from_raw`, `tokbuf.zig` `@ptrCast`, `interp` `unsafe`/`as_ptr`: `// SAFETY:` justification comments (cleared 2 of 3 `unsafe_block`). | | incidental | `tokbuf.zig`: added the owner-mandated `MPL-2.0` SPDX/Owner header the repo pre-commit hook requires (the file lacked it). | ### Residual (26 — all triaged, none critical) - **`from_raw` ×2 (high), `zig_ptr_cast` (high), `unsafe`/`as_ptr` (med):** accepted FFI/unsafe, now documented with `// SAFETY:` comments. (See FFI-proof follow-up below.) - **`expect_in_hot_path` ×8 (med):** accepted parser/typing **grammar invariants** (`expect()` on by-construction-unreachable cases). - **`SD022` ×7 / `SD009` ×4 (med):** doc-drift to pre-restructure paths, and missing SPDX on legacy `src/*.ml`. Declared in `.hypatia-ignore` + flagged to owner (SPDX edits are owner-gated; no auto-licence-edit). **⚠️ These remain visible because the current scanner emits `structural_drift` findings *without* routing them through `ScannerSuppression` — see follow-up.** - **`git_state` GS001/GS007 (med):** environmental (unstaged-tree state, which clears on commit; remote-branch-count policy). ### Two Hypatia scanner gaps found (worth upstream issues) 1. **`structural_drift` / `git_state` bypass `ScannerSuppression`** (`cli.ex` emits them directly), so `.hypatia-ignore` and inline directives can't suppress SD009/SD022/GS\* — inconsistent with `code_safety`. 2. **`secret_action_without_presence_gate` (WF017)** only recognizes a **step-level** `if:` gate, not a functionally-equivalent **job-level** one. ### FFI follow-up (answers a parallel question) `boj/boj-server` has a reusable zig↔idris safety framework directly applicable here: `ffi/zig/src/seams.zig` (32-test integration-contract harness, template-ready for `tokbuf.zig`), `src/abi/Boj/SafetyLemmas.idr` (`allRec` lemma family), and the `docs/proof-debt.md` + `docs/backend-assurance/` axiom + external-validation methodology — the principled path to retire the `@ptrCast`/`from_raw` findings via proof rather than suppression. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent c48fbde commit 7791b99

16 files changed

Lines changed: 114 additions & 6 deletions

File tree

.github/workflows/codeql.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ jobs:
3333
include:
3434
- language: javascript-typescript
3535
build-mode: none
36+
- language: actions
37+
build-mode: none
3638

3739
steps:
3840
- name: Checkout

.github/workflows/instant-sync.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ jobs:
2121
timeout-minutes: 5
2222
steps:
2323
- name: Trigger Propagation
24+
if: ${{ secrets.FARM_DISPATCH_TOKEN != '' }}
2425
uses: peter-evans/repository-dispatch@28959ce8df70de7be546dd1250a005dd32156697 # v3
2526
with:
2627
token: ${{ secrets.FARM_DISPATCH_TOKEN }}

.github/workflows/push-email-notify.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ jobs:
1313
name: Email on push
1414
if: ${{ vars.PUSH_EMAIL_ENABLED == 'true' }}
1515
runs-on: ubuntu-latest
16+
timeout-minutes: 5
1617
steps:
1718
- name: Send push notification email
1819
uses: dawidd6/action-send-mail@6e502825a508b867ab2954ad6343b68787624c01 # pinned

.github/workflows/scorecard.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,8 @@ permissions:
1313

1414
jobs:
1515
scorecard:
16+
permissions:
17+
contents: read
18+
security-events: write
19+
id-token: write
1620
uses: hyperpolymath/standards/.github/workflows/scorecard-reusable.yml@d135b05bfc647d0c0fbfedc7e80f37ea50f49236

.github/workflows/status-gate.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ jobs:
3838
status-gate-core:
3939
name: status-gate-core
4040
runs-on: ubuntu-latest
41+
timeout-minutes: 10
4142
steps:
4243
- name: Checkout repository
4344
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
@@ -49,6 +50,7 @@ jobs:
4950
status-gate-tests:
5051
name: status-gate-tests (slow · cancellable)
5152
runs-on: ubuntu-latest
53+
timeout-minutes: 30
5254
steps:
5355
- name: Notice — this is the slow, skippable job
5456
run: |

.hypatia-ignore

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,3 +30,47 @@
3030
cicd_rules/banned_language_file:formal/Makefile.coq
3131
cicd_rules/banned_language_file:formal/Makefile.coq.conf
3232
cicd_rules/banned_language_file:formal/Makefile.conf
33+
34+
# ─── Coq proof archaeology — legacy preservation admit (2026-06-24 triage) ─────
35+
#
36+
# formal/Semantics.v carries the LEGACY `preservation` Theorem (declared :8556),
37+
# closed with a DELIBERATE `Admitted.` (:9258). The theorem is provably FALSE —
38+
# formal/Counterexample.v discharges its falsity with 5 Qed lemmas — so the admit
39+
# is owner-mandated (CLAUDE.md 2026-05-27 fence + in-file ARCHAEOLOGY/DO-NOT-EXTEND
40+
# banner). It must never be "fixed"; the principled work is the four-layer redesign
41+
# in formal/PRESERVATION-DESIGN.md (preservation re-derived per layer). File-scoped:
42+
# the whole file is fenced legacy archaeology where any admit re-detection is the
43+
# same accepted non-defect. (Semantics_L1.v / Counterexample_L2_nested.v admits are
44+
# handled by in-file `hypatia: allow` directives, not here.)
45+
code_safety/admitted:formal/Semantics.v
46+
47+
# ─── Stale duplicate linter (lib/linter.rs) ────────────────────────────────────
48+
#
49+
# lib/linter.rs (178 lines) is a stale/legacy sibling of the live
50+
# src/ephapax-tools/src/linter.rs (920 lines). Its two `line.find(..).unwrap_or(0)+1`
51+
# sites compute 1-based diagnostic columns; 0 is a benign "column 1" default, not a
52+
# masked fault. Exempted pending an owner decision on whether to remove the duplicate.
53+
code_safety/unwrap_dangerous_default:lib/linter.rs
54+
55+
# ─── SD009: legacy OCaml files without SPDX headers (src/*.ml) ──────────────────
56+
#
57+
# src/{lexer,parser,ast,typecheck}.ml are legacy OCaml artefacts in a Rust/Coq/Idris
58+
# project (the live type checker is Rust). Adding licence headers is an SPDX edit,
59+
# which estate policy reserves for the owner (no-automated-licence-edits). Flag-only:
60+
# exempted here and surfaced to the owner — these are also candidates for removal.
61+
structural_drift/SD009:src/lexer.ml
62+
structural_drift/SD009:src/parser.ml
63+
structural_drift/SD009:src/ast.ml
64+
structural_drift/SD009:src/typecheck.ml
65+
66+
# ─── SD022: doc references to pre-restructure directories (deferred doc refresh) ─
67+
#
68+
# EXPLAINME/README/ROADMAP.adoc + tests/fuzz/FUZZING_GUIDE.md reference dirs
69+
# (src/core, src/Burble, src/Ephapax, src/parse_fuzzer, src/typecheck_fuzzer) that
70+
# predate the move to the `src/ephapax-*` cargo workspace. This is real (deferred)
71+
# doc-drift, not a false positive; exempted pending an owner doc-refresh
72+
# (prose = CC-BY-SA-4.0, not auto-edited in this security-triage PR).
73+
structural_drift/SD022:EXPLAINME.adoc
74+
structural_drift/SD022:README.adoc
75+
structural_drift/SD022:ROADMAP.adoc
76+
structural_drift/SD022:tests/fuzz/FUZZING_GUIDE.md

formal/Counterexample_L2_nested.v

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
(* SPDX-License-Identifier: MPL-2.0 *)
22
(* Owner: Jonathan D.A. Jewell <j.d.a.jewell@open.ac.uk> *)
33
(* SPDX-FileCopyrightText: 2026 Jonathan D.A. Jewell *)
4+
(* hypatia: allow code_safety/admitted -- FALSE POSITIVE: this file has no Coq admit/Admitted/Axiom; all proofs are Qed-closed. The scanner substring-matched prose in the doc comment that NEGATES their presence ("No new [Axiom] or [Admitted] markers"). *)
5+
(* hypatia: allow coq_admit_tactic -- same false positive: substring match on documentation prose, not a Coq tactic. *)
46

57
(** * Soundness gap (L2): preservation_l2 fails for nested-TFunEff
68
substituends — the gap Phase 3b Stage 1's [tfuneff_lambda_free]

formal/Semantics_L1.v

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
(* SPDX-License-Identifier: MPL-2.0 *)
22
(* SPDX-FileCopyrightText: 2026 Jonathan D.A. Jewell *)
33

4+
(* hypatia: allow code_safety/admitted -- TRACKED v2 proof-debt, not an honest-completion gap. The 3 Admitted lemmas (region_liveness_at_split_l1_gen, step_pop_disjoint_from_type_l1, preservation_l1) and their internal `admit.` tactics are documented in PROOF-NEEDS.md and owner-fenced by CLAUDE.md (directive 2026-05-27): closing or extending them is a banned anti-pattern. region_liveness is provably FALSE as stated; step_pop is open eliminator-fork research; preservation_l1 is the capstone gated on the others. Build truth is the `Print Assumptions` CI gate, not this comment. *)
5+
(* hypatia: allow coq_admit_tactic -- see preceding directive; same tracked admits. *)
6+
47
(**
58
69
*********************************************************************

idris2/ffi/zig/tokbuf.zig

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
// SPDX-License-Identifier: MPL-2.0
2+
// Owner: Jonathan D.A. Jewell <j.d.a.jewell@open.ac.uk>
3+
// SPDX-FileCopyrightText: 2026 Jonathan D.A. Jewell <j.d.a.jewell@open.ac.uk>
4+
15
const std = @import("std");
26

37
const Tok = struct {
@@ -85,6 +89,13 @@ export fn eph_tokbuf_str_ptr(buf: *TokBuf, idx: i32) [*:0]const u8 {
8589
const i = @as(usize, @intCast(idx));
8690
const t = buf.items[i];
8791
if (t.str_ptr) |p| {
92+
// SAFETY: p is [*]u8 (align 1); cast to the [*:0]const u8 return
93+
// type is alignment- and provenance-preserving and only narrows
94+
// mutability (adds const). The :0 sentinel is upheld by the
95+
// producer copyStr (allocates len+1 and writes a trailing NUL) and
96+
// required by the consumer (Idris FFI binds this to `String`, which
97+
// reads a NUL-terminated C string). The cast asserts, not checks,
98+
// the sentinel; the invariant is guaranteed at the copyStr seam.
8899
return @ptrCast(p);
89100
}
90101
return "";

src/ephapax-interp/src/lib.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,10 @@ impl Interpreter {
357357
/// Loading shared libraries is inherently unsafe. The caller must
358358
/// ensure the library is trustworthy.
359359
pub fn load_ffi_library(&mut self, path: &str) -> Result<(), RuntimeError> {
360+
// SAFETY: Loading a shared library is unsafe because the loaded code
361+
// is unverified; caller trust is documented in this fn's `# Safety`
362+
// section. The handle is stored in `self.ffi_libraries` and kept alive
363+
// until the interpreter is dropped, so no symbol outlives its library.
360364
let lib = unsafe {
361365
libloading::Library::new(path).map_err(|e| {
362366
RuntimeError::Unimplemented(format!("Failed to load FFI library '{}': {}", path, e))
@@ -373,6 +377,9 @@ impl Interpreter {
373377
) -> Option<libloading::Symbol<'a, unsafe extern "C" fn(i64, i64, i64, i64, i64, i64) -> i64>>
374378
{
375379
for lib in &self.ffi_libraries {
380+
// SAFETY: `lib` is a live, owned library handle borrowed from
381+
// `self.ffi_libraries` for `'a`; the returned Symbol is tied to that
382+
// borrow, so it cannot outlive the library it was resolved from.
376383
if let Ok(sym) = unsafe { lib.get(name.as_bytes()) } {
377384
return Some(sym);
378385
}
@@ -533,6 +540,11 @@ impl Interpreter {
533540
if let Some(func) = self.find_ffi_symbol(symbol) {
534541
// Pad args to 6 (max C ABI args we support)
535542
let a = |i: usize| *c_args.get(i).unwrap_or(&0);
543+
// SAFETY: `func` was resolved via find_ffi_symbol from a
544+
// loaded library to the 6-arg i64 C-ABI signature it is
545+
// called with here; args are padded to 6 so all params are
546+
// supplied. Soundness ultimately rests on the caller having
547+
// loaded a trustworthy library (see load_ffi_library).
536548
let result = unsafe { func(a(0), a(1), a(2), a(3), a(4), a(5)) };
537549
Ok(Value::I64(result))
538550
} else {
@@ -1233,6 +1245,10 @@ impl Interpreter {
12331245
// until the FFI call completes.
12341246
match std::ffi::CString::new(data.as_str()) {
12351247
Ok(cstr) => {
1248+
// SAFETY: the raw pointer stays valid because `cstr` is
1249+
// immediately moved into `cstrings` (kept alive until the
1250+
// FFI call completes) on the next line, so `ptr` never
1251+
// dangles for the duration of its use.
12361252
let ptr = cstr.as_ptr() as i64;
12371253
cstrings.push(cstr);
12381254
ptr

0 commit comments

Comments
 (0)