cb5def965b
Every file export buffered its output through a BufWriter and never
flushed it. BufWriter flushes on drop and DISCARDS any error it hits
doing so, so a write that fails only when the buffer is pushed to disk --
full disk, revoked permission, a network mount going away mid-export --
returned Ok(()). The status label then said "STL: 11 parts ->
model.stl" while the file on disk was truncated.
Demonstrated before fixing, with a writer that accepts buffered writes
and fails on flush:
without explicit flush -> Ok(())
with explicit flush -> Err("flush failed: disk full")
Three affected paths: PDF (exporters.rs), STL and GLB (workspace.rs).
All now go through exporters::export_to_file, which owns the BufWriter,
flushes it, and maps a failed flush to "<what>: could not write file:
<cause>". Putting the flush in one place is the point -- it was
forgotten three times independently.
The fourth BufWriter, in arch_pdf, writes into a Vec<u8> where flush
cannot fail. Left alone and excluded from the gate with that reason
recorded, rather than churned for uniformity.
Two tests pin the mechanism rather than one exporter: a small write
stays in the buffer, so write_all succeeds and only the flush can report
the failure. One asserts the drop path hides it, the other that an
explicit flush surfaces it. If BufWriter's drop behaviour ever changed,
the first test would fail and tell us the helper is no longer needed.
CI gate added and negative-tested: no BufWriter::new in the CAD module
outside the flushing helper.
Also checked, and found clean: arch_svg still escapes both XML sinks
after the Phase-4 projection rewrite, and all three cargo-deny
exemptions are still live -- removing them makes exactly those three
advisories fire, so none is a stale entry silently widening the policy.
723 lib + 154 integration, 0 failed. All twelve gates pass.
413 lines
18 KiB
YAML
413 lines
18 KiB
YAML
name: nigig-build (CAD)
|
|
|
|
# Phase 0.3 of the CAD remediation plan.
|
|
#
|
|
# Enforces:
|
|
# 1. Dependency resolution is pinned and reproducible (--locked).
|
|
# 2. Every git dependency is pinned to a rev, not a branch.
|
|
# 3. `nigig-build` compiles (lib + tests).
|
|
# 4. The test suite is green.
|
|
#
|
|
# History: the crate had 44 compile errors when this file was added, and
|
|
# then 17 failing tests once it built. Both are now fixed, so the test step
|
|
# asserts a plain pass instead of a "no worse than baseline" threshold.
|
|
|
|
on:
|
|
push:
|
|
paths:
|
|
- 'crates/apps/nigig-build/**'
|
|
- 'crates/apps/doc/**'
|
|
- 'crates/apps/spreadsheet/**'
|
|
- 'crates/nigig-core/**'
|
|
- 'crates/nigig-uikit/**'
|
|
- 'crates/matrix_client/**'
|
|
- 'Cargo.lock'
|
|
- 'Cargo.toml'
|
|
- 'rust-toolchain.toml'
|
|
- '.forgejo/workflows/nigig-build.yml'
|
|
pull_request:
|
|
paths:
|
|
- 'crates/apps/nigig-build/**'
|
|
- 'crates/apps/doc/**'
|
|
- 'crates/apps/spreadsheet/**'
|
|
- 'crates/nigig-core/**'
|
|
- 'crates/nigig-uikit/**'
|
|
- 'crates/matrix_client/**'
|
|
- 'Cargo.lock'
|
|
- 'Cargo.toml'
|
|
- 'rust-toolchain.toml'
|
|
- '.forgejo/workflows/nigig-build.yml'
|
|
|
|
jobs:
|
|
supply-chain:
|
|
runs-on: ubuntu-latest
|
|
timeout-minutes: 20
|
|
steps:
|
|
- uses: actions/checkout@v4
|
|
|
|
# A git dependency on a *branch* re-resolves on every build and is a
|
|
# direct code-execution path into CI if the branch is force-pushed.
|
|
# Phase 0.1 pinned all 33 manifests to an explicit rev; this keeps
|
|
# them pinned.
|
|
- name: Every git dependency must be pinned to a rev
|
|
run: |
|
|
set -euo pipefail
|
|
# Strip commented-out lines before checking; only live
|
|
# dependency declarations are in scope.
|
|
if grep -rn 'git = ' --include=Cargo.toml . \
|
|
| grep -v ':[0-9]*:[[:space:]]*#' \
|
|
| grep -v 'rev = '; then
|
|
echo
|
|
echo "ERROR: the git dependencies above are not pinned to a rev."
|
|
echo "Add rev = \"<full-40-char-sha>\" to each."
|
|
exit 1
|
|
fi
|
|
# ...and the rev must be the full 40-character SHA. An
|
|
# abbreviated rev resolves only while no other object shares
|
|
# its prefix; that is a property of the repository's current
|
|
# object count, not a guarantee. Git's own abbreviation length
|
|
# grows as a repo grows, so a short pin silently becomes
|
|
# ambiguous -- and an attacker who can push to the fork can
|
|
# try to manufacture a colliding prefix. This message has
|
|
# claimed "full-40-char-sha" since it was written without
|
|
# actually checking it.
|
|
if grep -rn 'rev = ' --include=Cargo.toml . \
|
|
| grep -v ':[0-9]*:[[:space:]]*#' \
|
|
| grep -vE 'rev = "[0-9a-f]{40}"'; then
|
|
echo
|
|
echo "ERROR: the rev(s) above are abbreviated. Use the full"
|
|
echo "40-character SHA so the pin cannot become ambiguous."
|
|
exit 1
|
|
fi
|
|
echo "OK: all git dependencies are pinned to a full SHA."
|
|
|
|
# A lockfile that changes during CI means the committed one was stale.
|
|
# An unused dependency is not cosmetic here. robius-sms was declared
|
|
# by nigig-build (and transitively by nigig-core and nigig-uikit)
|
|
# and never called once -- zero references in any of their sources.
|
|
# It dragged in polkit -> gio -> glib, which is the ONLY reason this
|
|
# crate needed RUSTSEC-2024-0370 and RUSTSEC-2024-0429 exemptions
|
|
# and carried an unanswered LGPL-2.1 distribution question. Deleting
|
|
# three manifest lines removed all of it.
|
|
#
|
|
# Scoped to the specific packages rather than a blanket
|
|
# `cargo machete`: five other unused dependencies exist across these
|
|
# crates (chrono, futures, postcard, rand, serde_json) and a gate
|
|
# that fails on day one gets switched off. Widen this list as those
|
|
# are cleared.
|
|
- name: The removed platform deps must not come back
|
|
run: |
|
|
set -euo pipefail
|
|
bad=0
|
|
for manifest in \
|
|
crates/apps/nigig-build/Cargo.toml \
|
|
crates/nigig-core/Cargo.toml \
|
|
crates/nigig-uikit/Cargo.toml; do
|
|
crate_dir="$(dirname "$manifest")"
|
|
for dep in robius-sms robius-location; do
|
|
declared=$(grep -cE "^[[:space:]]*${dep}[[:space:]]*=" "$manifest" || true)
|
|
[ "$declared" -eq 0 ] && continue
|
|
underscored="${dep//-/_}"
|
|
used=$(grep -rl "$underscored" "$crate_dir/src" 2>/dev/null | wc -l)
|
|
if [ "$used" -eq 0 ]; then
|
|
echo "ERROR: $manifest declares $dep but $crate_dir/src never uses it."
|
|
bad=1
|
|
fi
|
|
done
|
|
done
|
|
if [ "$bad" -ne 0 ]; then
|
|
echo
|
|
echo "These pull polkit/gio/glib into the graph and reintroduce"
|
|
echo "RUSTSEC-2024-0370, RUSTSEC-2024-0429 and an LGPL-2.1"
|
|
echo "distribution question, for code that is never called."
|
|
exit 1
|
|
fi
|
|
echo "OK"
|
|
|
|
- name: Lockfile must be committed and current
|
|
run: |
|
|
set -euo pipefail
|
|
test -f Cargo.lock || { echo "missing Cargo.lock at workspace root"; exit 1; }
|
|
cargo metadata --locked --format-version 1 > /dev/null
|
|
git diff --exit-code -- Cargo.lock
|
|
|
|
# Phase 2: these two classes of defect are easy to reintroduce by
|
|
# copy-paste and invisible in review.
|
|
- name: No build-time paths used at runtime in the CAD module
|
|
run: |
|
|
set -euo pipefail
|
|
cad=crates/apps/nigig-build/src/construction_frame/pages/workspace/cad
|
|
# Rust sources only. ARCHITECTURE.md documents this rule and so
|
|
# necessarily names the macro; scanning Markdown made the gate
|
|
# fail on its own documentation.
|
|
#
|
|
# Then strip the "file:line:" prefix and drop any line whose code
|
|
# starts with a comment marker. Doc/inline comments mentioning the
|
|
# macro are fine; a real call is not.
|
|
if grep -rn --include='*.rs' 'env!("CARGO_MANIFEST_DIR")' "$cad" \
|
|
| sed 's/^[^:]*:[0-9]*://' \
|
|
| grep -vE '^[[:space:]]*(//|/\*|\*)'; then
|
|
echo
|
|
echo "ERROR: CARGO_MANIFEST_DIR is a BUILD-time path. Using it at"
|
|
echo "runtime bakes the build machine's source tree into the"
|
|
echo "binary. Use persistence::cad_data_dir() instead."
|
|
exit 1
|
|
fi
|
|
echo "OK"
|
|
|
|
- name: No hardcoded network endpoints in the CAD module
|
|
run: |
|
|
set -euo pipefail
|
|
cad=crates/apps/nigig-build/src/construction_frame/pages/workspace/cad
|
|
# RFC1918 literals outside comments. constants.rs is excluded
|
|
# wholesale: its only matches are the endpoint_tests that assert
|
|
# such addresses are REJECTED.
|
|
if grep -rnE --include='*.rs' \
|
|
'"https?://(10\.|192\.168\.|172\.(1[6-9]|2[0-9]|3[01])\.)' "$cad" \
|
|
--exclude=constants.rs \
|
|
| sed 's/^[^:]*:[0-9]*://' \
|
|
| grep -vE '^[[:space:]]*(//|/\*|\*)'; then
|
|
echo
|
|
echo "ERROR: a private-network endpoint is hardcoded above."
|
|
echo "Read it from the environment (see constants::local_openai_url)."
|
|
exit 1
|
|
fi
|
|
echo "OK"
|
|
|
|
# CadNode::pos/rot/size return Vec3f BY VALUE. `node.pos().x = v`
|
|
# compiles, mutates a temporary and throws it away. This silently
|
|
# broke all nine properties-panel inputs and the Extend tool; it
|
|
# produces no warning and no runtime error, only a control that
|
|
# does nothing. Mutation must go through set_pos/set_rot/set_size.
|
|
- name: No writes through the by-value Vec3f getters
|
|
run: |
|
|
set -euo pipefail
|
|
cad=crates/apps/nigig-build/src/construction_frame/pages/workspace/cad
|
|
# Exclude the two test modules that demonstrate the trap.
|
|
if grep -rnE --include='*.rs' \
|
|
'\.(pos|rot|size)\(\)\.[xyz][[:space:]]*[-+*/]?=[^=]' "$cad" \
|
|
| grep -v 'setters_write_through_but_getters_are_copies' \
|
|
| sed 's/^[^:]*:[0-9]*://' \
|
|
| grep -vE '^[[:space:]]*(//|/\*|\*)' \
|
|
| grep -v 'n\.pos()\.x = 42\.0' \
|
|
| grep -v 'n\.rot()\.y = 42\.0' \
|
|
| grep -v 'p\.size()\.y = v'; then
|
|
echo
|
|
echo "ERROR: the assignment(s) above write to a temporary copy"
|
|
echo "and are discarded. CadNode::pos/rot/size return Vec3f by"
|
|
echo "value. Read into a local, mutate it, then call set_pos /"
|
|
echo "set_rot / set_size."
|
|
exit 1
|
|
fi
|
|
echo "OK"
|
|
|
|
# eval_cad_script_in_vm is the single chokepoint where user- and
|
|
# AI-authored source reaches the script VM, and vm.eval is one
|
|
# blocking call. Without a run budget an unterminated loop wedges the
|
|
# rebuild worker permanently -- reproduced before the budget existed:
|
|
# still running after 90 seconds. Losing this line would reintroduce
|
|
# a hang that no test failure announces, only a frozen app.
|
|
- name: The CAD script evaluator must set a run budget
|
|
run: |
|
|
set -euo pipefail
|
|
f=crates/apps/nigig-build/src/construction_frame/pages/workspace/cad/script_bindings.rs
|
|
if ! grep -q 'vm.bx.run_budget = Some(' "$f"; then
|
|
echo "ERROR: eval_cad_script_in_vm no longer installs a"
|
|
echo "ScriptRunBudget. An unterminated CAD script would hang the"
|
|
echo "rebuild worker forever. See CAD_SCRIPT_TIME_BUDGET."
|
|
exit 1
|
|
fi
|
|
echo "OK"
|
|
|
|
# Phase 0.4. Blocked until Cargo.lock was committed in Phase 0.2,
|
|
# because cargo-deny resolves the graph from the lockfile.
|
|
#
|
|
# Gates security advisories and yanked crates. The licence, bans and
|
|
# source checks also run, but are configured to reflect what this
|
|
# graph actually is -- see deny-nigig-build.toml, where every
|
|
# exception is named and justified rather than blanket-disabled. A
|
|
# NEW unlicensed crate, or a new advisory, still fails.
|
|
- name: Dependency audit, licences, bans and sources
|
|
run: |
|
|
set -euo pipefail
|
|
version=0.18.6
|
|
url="https://github.com/EmbarkStudios/cargo-deny/releases/download"
|
|
curl -sSLf -o /tmp/cargo-deny.tar.gz \
|
|
"$url/$version/cargo-deny-$version-x86_64-unknown-linux-musl.tar.gz"
|
|
tar xzf /tmp/cargo-deny.tar.gz -C /tmp
|
|
install -m 0755 \
|
|
"/tmp/cargo-deny-$version-x86_64-unknown-linux-musl/cargo-deny" \
|
|
/usr/local/bin/cargo-deny
|
|
# --config is resolved relative to the manifest, not the working
|
|
# directory, so it must be absolute.
|
|
cargo-deny --manifest-path crates/apps/nigig-build/Cargo.toml \
|
|
--all-features check --config "$PWD/deny-nigig-build.toml"
|
|
|
|
# A bare identifier in a match pattern that is NOT a known variant
|
|
# is parsed as a new binding that matches everything. Four such
|
|
# names (KeyEnter, KeyBackspace, BracketLeft, BracketRight) plus
|
|
# Digit0-9/Equal/LeftBracket/RightBracket/Apostrophe silently turned
|
|
# keyboard handlers into catch-alls: the whole direct-distance-entry
|
|
# feature was unreachable, and typing any digit produced '0'.
|
|
#
|
|
# It compiles, and no test catches it. rustc reports it as
|
|
# `unreachable_pattern` plus an `unused_variables` warning on a
|
|
# capitalised name -- which is the signature grepped for here.
|
|
- name: No match arms binding a non-existent enum variant
|
|
run: |
|
|
set -euo pipefail
|
|
# A capitalised "unused variable" is almost always a mistyped
|
|
# variant used as a pattern.
|
|
out=$(cargo build --locked -p nigig-build --lib --message-format=short 2>&1 \
|
|
| grep -E 'unused variable: `[A-Z]' || true)
|
|
if [ -n "$out" ]; then
|
|
echo "$out"
|
|
echo
|
|
echo "ERROR: the pattern(s) above bind a new variable instead of"
|
|
echo "matching an enum variant -- check the spelling against the"
|
|
echo "enum definition. These silently swallow every input."
|
|
exit 1
|
|
fi
|
|
echo "OK"
|
|
|
|
# `save_cad_script(..).ok();` compiles, discards a real io::Error,
|
|
# and used to be followed by an unconditional "Saved" label -- the
|
|
# user was told their work was safe when the write had failed. A
|
|
# save path is the one place a dropped Result is data loss.
|
|
- name: No discarded Results on CAD save/write paths
|
|
run: |
|
|
set -euo pipefail
|
|
cad=crates/apps/nigig-build/src/construction_frame/pages/workspace/cad
|
|
if grep -rnE --include='*.rs' \
|
|
'(save|write|persist|store|export)[A-Za-z_]*\([^;]*\)\.ok\(\);' \
|
|
"$cad" \
|
|
| sed 's/^[^:]*:[0-9]*://' \
|
|
| grep -vE '^[[:space:]]*(//|/\*|\*)'; then
|
|
echo
|
|
echo "ERROR: the call(s) above throw away a Result from a write"
|
|
echo "path. Surface it -- see save_status_message()."
|
|
exit 1
|
|
fi
|
|
echo "OK"
|
|
|
|
# A CAD editor holds unsaved work. A panic on a UI path takes the
|
|
# whole model with it, which is strictly worse than the mistake
|
|
# being reported. `Command::merge`'s default panicked on a
|
|
# reachable path -- a command overriding can_merge but not merge --
|
|
# while the code five lines away already refused to panic on an
|
|
# unreachable one. Use debug_assert!, or return.
|
|
- name: No panicking macros in CAD production code
|
|
run: |
|
|
set -euo pipefail
|
|
cad=crates/apps/nigig-build/src/construction_frame/pages/workspace/cad
|
|
hits=0
|
|
for f in "$cad"/*.rs; do
|
|
# Stop at the first #[cfg(test)]: test code may panic freely.
|
|
out=$(awk '/^#\[cfg\(test\)\]/{exit}
|
|
/unreachable!\(|panic!\(|todo!\(|unimplemented!\(/{
|
|
printf "%d: %s\n", NR, $0 }' "$f" \
|
|
| grep -vE '^[0-9]+:[[:space:]]*(//|/\*|\*)' || true)
|
|
if [ -n "$out" ]; then
|
|
echo "$f"; echo "$out"; hits=1
|
|
fi
|
|
done
|
|
if [ "$hits" -ne 0 ]; then
|
|
echo
|
|
echo "ERROR: the panicking macro(s) above are in production code."
|
|
echo "Prefer debug_assert! (loud in dev, survivable in release)"
|
|
echo "or an early return."
|
|
exit 1
|
|
fi
|
|
echo "OK"
|
|
|
|
# BufWriter flushes on drop and DISCARDS any error it hits. Every
|
|
# CAD export buffered its output, so a write that failed only at
|
|
# flush time -- full disk, revoked permission, network mount gone --
|
|
# returned Ok(()) and the status label said the file was written.
|
|
# Route file exports through exporters::export_to_file, which
|
|
# flushes and reports.
|
|
- name: No unflushed BufWriter in CAD export paths
|
|
run: |
|
|
set -euo pipefail
|
|
cad=crates/apps/nigig-build/src/construction_frame/pages/workspace/cad
|
|
# exporters.rs owns the one legitimate BufWriter (inside
|
|
# export_to_file, which flushes). arch_pdf writes into a Vec,
|
|
# where flush cannot fail.
|
|
if grep -rnE --include='*.rs' 'BufWriter::new' "$cad" \
|
|
--exclude=exporters.rs --exclude=arch_pdf.rs \
|
|
| sed 's/^[^:]*:[0-9]*://' \
|
|
| grep -vE '^[[:space:]]*(//|/\*|\*)'; then
|
|
echo
|
|
echo "ERROR: the BufWriter(s) above are outside the flushing"
|
|
echo "helper. Use exporters::export_to_file so a failed flush"
|
|
echo "is reported instead of silently truncating the file."
|
|
exit 1
|
|
fi
|
|
echo "OK"
|
|
|
|
- name: Reject whitespace errors
|
|
run: git diff --check
|
|
|
|
cad-module:
|
|
runs-on: ubuntu-latest
|
|
timeout-minutes: 45
|
|
steps:
|
|
- uses: actions/checkout@v4
|
|
|
|
# Makepad needs a desktop/GL stack even for a check build.
|
|
- name: Install native dependencies
|
|
run: |
|
|
sudo apt-get update -qq
|
|
sudo apt-get install -y -qq \
|
|
pkg-config libwayland-dev libxcursor-dev libxrandr-dev \
|
|
libxi-dev libx11-dev libgl1-mesa-dev libasound2-dev \
|
|
libpolkit-gobject-1-dev libpolkit-agent-1-dev \
|
|
libglib2.0-dev libssl-dev libsqlite3-dev libudev-dev
|
|
|
|
- name: Formatting (CAD module)
|
|
run: |
|
|
cargo fmt --version
|
|
cargo fmt -p nigig-build -- --check \
|
|
|| { echo "run: cargo fmt -p nigig-build"; exit 1; }
|
|
|
|
full-crate-check:
|
|
runs-on: ubuntu-latest
|
|
timeout-minutes: 60
|
|
steps:
|
|
- uses: actions/checkout@v4
|
|
|
|
# libpulse/libxkbcommon are link-time-only: `cargo check` passes
|
|
# without them, `cargo test` does not.
|
|
- name: Install native dependencies
|
|
run: |
|
|
sudo apt-get update -qq
|
|
sudo apt-get install -y -qq \
|
|
pkg-config libwayland-dev libxcursor-dev libxrandr-dev \
|
|
libxi-dev libx11-dev libgl1-mesa-dev libasound2-dev \
|
|
libpolkit-gobject-1-dev libpolkit-agent-1-dev \
|
|
libglib2.0-dev libssl-dev libsqlite3-dev libudev-dev \
|
|
libpulse-dev libxkbcommon-dev
|
|
|
|
- name: Check
|
|
run: cargo check --locked -p nigig-build --lib
|
|
|
|
- name: Test (lib)
|
|
run: cargo test --locked -p nigig-build --lib
|
|
|
|
# Phase 4.7 moved the CAD integration suite out of src/ into its own
|
|
# test target. `--lib` does not build it, so without this step the
|
|
# 154 tests it contains would run in no pipeline at all.
|
|
#
|
|
# This names the target explicitly rather than running the whole
|
|
# crate's tests, because `--test cost_estimator` and
|
|
# `--test cost_estimator_ui` do not currently compile. That is
|
|
# pre-existing breakage, not this workflow's to hide -- but gating
|
|
# on it would make this job red for reasons unrelated to the CAD
|
|
# module, and a step that is always red gets ignored. Add those
|
|
# targets here the moment they build.
|
|
- name: Test (CAD integration suite)
|
|
run: cargo test --locked -p nigig-build --test cad_integration
|
|
|
|
# NOTE: `cargo clippy -- -D warnings` is not enabled yet -- the crate
|
|
# currently emits ~290 warnings. Enable it once that backlog is
|
|
# cleared; a step that cannot fail is worse than no step.
|