fix(ci): fail closed on skipped security e2e and gate frontend tests #187

New issue

Open

opened 2026-09-12 12:58:53 +00:00 by PlasticDigits · 2 comments

PlasticDigits commented

2026-09-12 12:58:53 +00:00

Owner

Summary

Security-critical CI is fail-open in four places on main (2026-09-12). Bundle them; do not split.

packages/e2e treats TestResult::Skip as success. cl8y-e2e run / full exit 1 only when TestSuite::failed() > 0. Skip is printed, never fatal.
.github/workflows/e2e.yml still chmod +x ./scripts/e2e-test.sh then runs it. That path is gone from scripts/ (Rust cl8y-e2e replaced it; README points at scripts/legacy/). The same file’s header says “ALL tests run on EVERY push / Do NOT add flags that skip tests.”
Woodpecker is the required Forgejo check (.woodpecker.yaml from #172 / #174). The frontend-hash-verify step runs three Vitest files only (hashVerifyExecuteBlocker, HashComparisonPanel, DestHashCard). packages/frontend has npm run test:unit (SKIP_INTEGRATION=true vitest run) plus a large Playwright tree under e2e/. .github/workflows/test.yml does not even list packages/frontend/**.
Canceler integration tests in packages/canceler/tests/integration_test.rs stay #[ignore = "requires Anvil…/LocalTerra…"]. e2e.yml already starts Anvil, LocalTerra, and Postgres, then cargo test --release -- --nocapture without --ignored. should_run_integration() is true when CI is set, but #[ignore] still wins unless --ignored is passed.

This is not #172 (Woodpecker enablement, operator redeploy, hash-verify minimum). Do not reopen or comment that ticket for this. This is not #186 (pipe-to-shell Solana installer). This is not #98 (Bridge vs AccessManager role IDs).

Internal review id: TEST-1 (high). Still in source 2026-09-12.

Repro

Given packages/e2e TestSuite and Commands::Run / Full in src/main.rs
When any test returns TestResult::skip(...) (missing env, --no-terra, unset SOLANA_ENABLED, missing contract address)
Then the process still exits 0 if failed() == 0

Given .github/workflows/e2e.yml step Run Full E2E Test Suite (Bash)
When that step runs on a current checkout
Then chmod +x ./scripts/e2e-test.sh fails because the file is not in scripts/

Given a PR that changes frontend unit tests outside the three hash-verify files
When Woodpecker is the required check
Then npm run test:unit of those files never runs; Playwright never runs

Given e2e.yml has started Anvil + LocalTerra
When the canceler unit-test step runs
Then #[ignore] integration tests are not executed

Expected vs actual

Surface	Expected	Actual
Security-critical `cl8y-e2e`	Skip is a job failure (or an explicit allowlist of non-security probes)	Skip is yellow in the summary; exit 0
Bash E2E step	Script exists or the step is deleted in favor of `cargo run -p cl8y-e2e`	Dead `scripts/e2e-test.sh`
Frontend on PRs	Full `test:unit` + one Playwright smoke as a required Woodpecker step	Three hash-verify files only
Canceler integration	Run where Anvil/LocalTerra already exist	Ignored forever in that job

Why this is needed

A skipped fraud / canceler / operator-reject / hash-parity case looks like a green pipeline. The workflow comments claim the opposite. Frontend regressions outside Hash Verification never gate merge. Canceler watchtower paths that need Anvil/LocalTerra are documented as “run with --ignored” and then never invoked in the job that has that infra.

No contract, operator, or frontend runtime change. CI + test harness only.

Constraints / guardrails

Fail closed on security-critical skips only. Allowlist (must be named in code, not a comment): optional Solana live flows when SOLANA_ENABLED is unset and the README already says GitHub e2e.yml does not enable them; --quick connectivity-only mode when explicitly requested. Do not allowlist fraud_detection*, canceler_*, operator_rejects_unverified*, operator_no_approve_solana, xchain_hash_*, watchtower / cancel-window cases.
Prefer if suite.failed() > 0 || security_skips > 0 { exit 1 } (or !suite.all_passed() for the security subset). Keep TestResult::Skip for the allowlist.
Delete or retarget the bash E2E step. Do not restore scripts/e2e-test.sh as a second runner. packages/e2e/README.md already maps it to cargo run -p cl8y-e2e -- run.
Do not add --no-terra / --no-operator / --no-canceler to the full CI job.
Woodpecker: keep digest-pinned images from #172 / #174. Do not drop gitleaks or operator-writers. Expand frontend-hash-verify to npm run test:unit with no path filter. Playwright smoke must be E2E_UI_ONLY=1 (no Docker compose, no contract deploy) — e.g. e2e/wallet-connect.spec.ts or e2e/validation.spec.ts with a documented project. Full transfer Playwright stays out of Woodpecker in this ticket.
Canceler: do not remove #[ignore] so cargo test on a laptop without Anvil stays green. In jobs that already have Anvil + LocalTerra (at least e2e.yml), run cargo test --test integration_test -- --ignored --nocapture with INTEGRATION_TEST=1 and the bridge addresses that job already exports. Skip a test with a hard failure if required EVM_BRIDGE_ADDRESS / TERRA_BRIDGE_ADDRESS is missing in that job (no silent ignore).
Do not fold #186 Solana installer pins or #185 Compose image pins into this PR.
Do not publish live RPC URLs, tokens, or operator logs.

Relevant files

Path	Why
`packages/e2e/src/lib.rs`	`TestResult::Skip`; `TestSuite::failed` / `skipped` / `all_passed`
`packages/e2e/src/main.rs`	Exit `1` only if `failed() > 0`; `--no-terra` skip path
`packages/e2e/src/tests/`	Callers of `TestResult::skip`
`.github/workflows/e2e.yml`	Dead `scripts/e2e-test.sh`; canceler `cargo test` without `--ignored`
`.woodpecker.yaml`	Required check; three-file Vitest filter
`packages/frontend/package.json`	`test:unit`, `test:e2e`
`packages/frontend/e2e/`	Playwright specs; smoke candidate
`packages/frontend/playwright.config.ts`	`E2E_UI_ONLY` / `CI` teardown
`packages/canceler/tests/integration_test.rs`	`#[ignore]` integration suite
`docs/testing.md`	Still documents `./scripts/e2e-test.sh` as the master suite

Recommended direction

Classify each TestResult::skip as security-critical vs allowlisted. Security skip → fail the process. Unit-test the classifier with a fixture suite (no chain).
Remove the bash E2E step from e2e.yml (or replace with cargo run -p cl8y-e2e -- run which the job already ran). Grep CI YAML for e2e-test.sh.
Woodpecker frontend-hash-verify: npm ci then npm run test:unit (all unit files). New step frontend-playwright-smoke: npx playwright install --with-deps chromium (or the pinned browsers the config already uses) then E2E_UI_ONLY=1 npx playwright test <smoke-spec> --workers=1. Digest-pin the Node image already used.
e2e.yml canceler step: after contracts exist, cargo test --test integration_test --release -- --ignored --nocapture. Keep the default cargo test (non-ignored) for unit tests.

Acceptance criteria

AC1. cl8y-e2e run exits ≠ 0 if any security-critical test is Skip, even when failed() == 0.
AC2. Allowlisted skips (documented optional Solana live; explicit --quick) still exit 0 when everything else passed.
AC3. .github/workflows/e2e.yml has no scripts/e2e-test.sh. Grep of .github/workflows and .woodpecker.yaml finds no live reference except maybe scripts/legacy/ docs.
AC4. Woodpecker runs full packages/frontend npm run test:unit (not a three-file filter) on PR/push.
AC5. Woodpecker runs one Playwright smoke with E2E_UI_ONLY=1 (no compose). Failure fails the pipeline.
AC6. e2e.yml (or equivalent job that already has Anvil + LocalTerra) runs canceler integration_test with --ignored. Missing required bridge env fails that step.
AC7. cd packages/canceler && cargo test without --ignored still skips integration tests (laptop without infra).
AC8. docs/testing.md no longer presents ./scripts/e2e-test.sh as the live runner; it points at cl8y-e2e and the Woodpecker frontend steps.
AC9. No on-chain / operator / frontend product behavior change.

Test plan (functional paths)

#	Path	Expect
T1	Fixture suite: one security skip, zero fails	Process exit ≠ 0
T2	Fixture suite: all pass	Exit 0
T3	Fixture suite: allowlisted skip only	Exit 0
T4	`grep -R e2e-test.sh .github .woodpecker.yaml`	No workflow hit
T5	Woodpecker frontend unit step	`vitest run` without the three-file argv
T6	Playwright smoke `E2E_UI_ONLY=1`	Spec runs; no `docker compose up`
T7	Canceler `cargo test -- --ignored` with Anvil up	`test_anvil_connectivity` runs (not silently ignored)
T8	Canceler `cargo test` (no `--ignored`)	Integration tests skipped
T9	`docs/testing.md`	No “run `./scripts/e2e-test.sh`” as current CI

Test plan (attack, hack, and abuse)

Non-exploitative. CI fixtures only. These catch fail-open test gating, not a drain recipe.

#	Vector	Expect
A1	Skip `fraud_detection` because env is empty	Job red
A2	Skip `canceler_solana_source_fraud` because `SOLANA_ENABLED` unset if classified security-critical	Job red (or the test is not in the default required set — then it must not be counted as coverage)
A3	Reintroduce `chmod +x ./scripts/e2e-test.sh`	Grep / CI check fails
A4	Woodpecker path-filter back to the three hash-verify files	Review fail; unit step must be unfiltered `test:unit`
A5	Playwright smoke without `E2E_UI_ONLY` pulling compose on Woodpecker	Out of scope / forbidden in this ticket
A6	`cargo test` in canceler job without `--ignored` as the only canceler command	Fail AC6
A7	Comment in YAML “NO SKIPPING” while skip is still success	Code path A1 still fails; comment is not the control

Verification criteria

Unit tests around TestSuite exit policy (T1–T3) in packages/e2e.
Workflow grep for e2e-test.sh.
Woodpecker YAML: npm run test:unit with no extra test-file argv; Playwright smoke env E2E_UI_ONLY=1.
Canceler job log shows ignored tests executed when infra is up (running 1 test for an Anvil connectivity case), not ignored.
Do not verify by skipping a live fraud case on production RPC.

Out of scope

#172 operator live execute / Woodpecker enablement.
#186 Solana CLI pipe-to-shell.
#185 Compose :latest digest pins (including e2e.yml localterra-cl8y:latest).
Moving Foundry / Anchor / full cl8y-e2e full onto Woodpecker.
Full Playwright transfer matrix (transfer-*-to-*.spec.ts) on Woodpecker.
Un-ignoring operator packages/operator/tests/integration_test.rs (sibling ignore; only if the same e2e.yml step is a one-line add — not required to close TEST-1).
Changing canceler/operator runtime logic.

First-pass model recommendation

Recommendation: grok-high

Rationale: Cross-cutting CI policy across packages/e2e (exit policy), .github/workflows/e2e.yml (dead script + canceler --ignored), .woodpecker.yaml (frontend unit + Playwright smoke), canceler integration attributes, and docs/testing.md. More than three production files and more than one subsystem. Woodpecker is the required merge check — a wrong skip allowlist or a Playwright step that pulls compose will either hide security tests again or break every PR. File count does not make this Composer-eligible. Verify with T1–T3 fixtures, workflow greps, and a Woodpecker PR that fails on a deliberate security skip / omitted unit file — not by asserting a green pipeline that skipped the suite.

## Summary Security-critical CI is fail-open in four places on `main` (2026-09-12). Bundle them; do not split. 1. `packages/e2e` treats `TestResult::Skip` as success. `cl8y-e2e run` / `full` exit `1` only when `TestSuite::failed() > 0`. Skip is printed, never fatal. 2. `.github/workflows/e2e.yml` still `chmod +x ./scripts/e2e-test.sh` then runs it. That path is gone from `scripts/` (Rust `cl8y-e2e` replaced it; README points at `scripts/legacy/`). The same file’s header says “ALL tests run on EVERY push / Do NOT add flags that skip tests.” 3. Woodpecker is the required Forgejo check (`.woodpecker.yaml` from #172 / #174). The `frontend-hash-verify` step runs three Vitest files only (`hashVerifyExecuteBlocker`, `HashComparisonPanel`, `DestHashCard`). `packages/frontend` has `npm run test:unit` (`SKIP_INTEGRATION=true vitest run`) plus a large Playwright tree under `e2e/`. `.github/workflows/test.yml` does not even list `packages/frontend/**`. 4. Canceler integration tests in `packages/canceler/tests/integration_test.rs` stay `#[ignore = "requires Anvil…/LocalTerra…"]`. `e2e.yml` already starts Anvil, LocalTerra, and Postgres, then `cargo test --release -- --nocapture` **without** `--ignored`. `should_run_integration()` is true when `CI` is set, but `#[ignore]` still wins unless `--ignored` is passed. This is not #172 (Woodpecker enablement, operator redeploy, hash-verify **minimum**). Do not reopen or comment that ticket for this. This is not #186 (pipe-to-shell Solana installer). This is not #98 (Bridge vs AccessManager role IDs). Internal review id: TEST-1 (high). Still in source 2026-09-12. ### Repro **Given** `packages/e2e` `TestSuite` and `Commands::Run` / `Full` in `src/main.rs` **When** any test returns `TestResult::skip(...)` (missing env, `--no-terra`, unset `SOLANA_ENABLED`, missing contract address) **Then** the process still exits 0 if `failed() == 0` **Given** `.github/workflows/e2e.yml` step `Run Full E2E Test Suite (Bash)` **When** that step runs on a current checkout **Then** `chmod +x ./scripts/e2e-test.sh` fails because the file is not in `scripts/` **Given** a PR that changes frontend unit tests outside the three hash-verify files **When** Woodpecker is the required check **Then** `npm run test:unit` of those files never runs; Playwright never runs **Given** `e2e.yml` has started Anvil + LocalTerra **When** the canceler unit-test step runs **Then** `#[ignore]` integration tests are not executed ### Expected vs actual | Surface | Expected | Actual | | --- | --- | --- | | Security-critical `cl8y-e2e` | Skip is a job failure (or an explicit allowlist of non-security probes) | Skip is yellow in the summary; exit 0 | | Bash E2E step | Script exists **or** the step is deleted in favor of `cargo run -p cl8y-e2e` | Dead `scripts/e2e-test.sh` | | Frontend on PRs | Full `test:unit` + one Playwright smoke as a required Woodpecker step | Three hash-verify files only | | Canceler integration | Run where Anvil/LocalTerra already exist | Ignored forever in that job | ## Why this is needed A skipped fraud / canceler / operator-reject / hash-parity case looks like a green pipeline. The workflow comments claim the opposite. Frontend regressions outside Hash Verification never gate merge. Canceler watchtower paths that need Anvil/LocalTerra are documented as “run with `--ignored`” and then never invoked in the job that has that infra. No contract, operator, or frontend **runtime** change. CI + test harness only. ## Constraints / guardrails - Fail closed on **security-critical** skips only. Allowlist (must be named in code, not a comment): optional Solana live flows when `SOLANA_ENABLED` is unset and the README already says GitHub `e2e.yml` does not enable them; `--quick` connectivity-only mode when explicitly requested. Do not allowlist `fraud_detection*`, `canceler_*`, `operator_rejects_unverified*`, `operator_no_approve_solana`, `xchain_hash_*`, watchtower / cancel-window cases. - Prefer `if suite.failed() > 0 || security_skips > 0 { exit 1 }` (or `!suite.all_passed()` for the security subset). Keep `TestResult::Skip` for the allowlist. - Delete or retarget the bash E2E step. Do not restore `scripts/e2e-test.sh` as a second runner. `packages/e2e/README.md` already maps it to `cargo run -p cl8y-e2e -- run`. - Do not add `--no-terra` / `--no-operator` / `--no-canceler` to the full CI job. - Woodpecker: keep digest-pinned images from #172 / #174. Do not drop gitleaks or `operator-writers`. Expand `frontend-hash-verify` to `npm run test:unit` with **no** path filter. Playwright smoke must be `E2E_UI_ONLY=1` (no Docker compose, no contract deploy) — e.g. `e2e/wallet-connect.spec.ts` or `e2e/validation.spec.ts` with a documented project. Full transfer Playwright stays out of Woodpecker in this ticket. - Canceler: do **not** remove `#[ignore]` so `cargo test` on a laptop without Anvil stays green. In jobs that already have Anvil + LocalTerra (at least `e2e.yml`), run `cargo test --test integration_test -- --ignored --nocapture` with `INTEGRATION_TEST=1` and the bridge addresses that job already exports. Skip a test with a hard failure if required `EVM_BRIDGE_ADDRESS` / `TERRA_BRIDGE_ADDRESS` is missing in that job (no silent ignore). - Do not fold #186 Solana installer pins or #185 Compose image pins into this PR. - Do not publish live RPC URLs, tokens, or operator logs. ## Relevant files | Path | Why | | --- | --- | | `packages/e2e/src/lib.rs` | `TestResult::Skip`; `TestSuite::failed` / `skipped` / `all_passed` | | `packages/e2e/src/main.rs` | Exit `1` only if `failed() > 0`; `--no-terra` skip path | | `packages/e2e/src/tests/` | Callers of `TestResult::skip` | | `.github/workflows/e2e.yml` | Dead `scripts/e2e-test.sh`; canceler `cargo test` without `--ignored` | | `.woodpecker.yaml` | Required check; three-file Vitest filter | | `packages/frontend/package.json` | `test:unit`, `test:e2e` | | `packages/frontend/e2e/` | Playwright specs; smoke candidate | | `packages/frontend/playwright.config.ts` | `E2E_UI_ONLY` / `CI` teardown | | `packages/canceler/tests/integration_test.rs` | `#[ignore]` integration suite | | `docs/testing.md` | Still documents `./scripts/e2e-test.sh` as the master suite | ## Recommended direction 1. Classify each `TestResult::skip` as security-critical vs allowlisted. Security skip → fail the process. Unit-test the classifier with a fixture suite (no chain). 2. Remove the bash E2E step from `e2e.yml` (or replace with `cargo run -p cl8y-e2e -- run` which the job already ran). Grep CI YAML for `e2e-test.sh`. 3. Woodpecker `frontend-hash-verify`: `npm ci` then `npm run test:unit` (all unit files). New step `frontend-playwright-smoke`: `npx playwright install --with-deps chromium` (or the pinned browsers the config already uses) then `E2E_UI_ONLY=1 npx playwright test <smoke-spec> --workers=1`. Digest-pin the Node image already used. 4. `e2e.yml` canceler step: after contracts exist, `cargo test --test integration_test --release -- --ignored --nocapture`. Keep the default `cargo test` (non-ignored) for unit tests. ## Acceptance criteria - AC1. `cl8y-e2e run` exits ≠ 0 if any security-critical test is `Skip`, even when `failed() == 0`. - AC2. Allowlisted skips (documented optional Solana live; explicit `--quick`) still exit 0 when everything else passed. - AC3. `.github/workflows/e2e.yml` has no `scripts/e2e-test.sh`. Grep of `.github/workflows` and `.woodpecker.yaml` finds no live reference except maybe `scripts/legacy/` docs. - AC4. Woodpecker runs full `packages/frontend` `npm run test:unit` (not a three-file filter) on PR/push. - AC5. Woodpecker runs one Playwright smoke with `E2E_UI_ONLY=1` (no compose). Failure fails the pipeline. - AC6. `e2e.yml` (or equivalent job that already has Anvil + LocalTerra) runs canceler `integration_test` with `--ignored`. Missing required bridge env fails that step. - AC7. `cd packages/canceler && cargo test` without `--ignored` still skips integration tests (laptop without infra). - AC8. `docs/testing.md` no longer presents `./scripts/e2e-test.sh` as the live runner; it points at `cl8y-e2e` and the Woodpecker frontend steps. - AC9. No on-chain / operator / frontend product behavior change. ## Test plan (functional paths) | # | Path | Expect | | --- | --- | --- | | T1 | Fixture suite: one security skip, zero fails | Process exit ≠ 0 | | T2 | Fixture suite: all pass | Exit 0 | | T3 | Fixture suite: allowlisted skip only | Exit 0 | | T4 | `grep -R e2e-test.sh .github .woodpecker.yaml` | No workflow hit | | T5 | Woodpecker frontend unit step | `vitest run` without the three-file argv | | T6 | Playwright smoke `E2E_UI_ONLY=1` | Spec runs; no `docker compose up` | | T7 | Canceler `cargo test -- --ignored` with Anvil up | `test_anvil_connectivity` runs (not silently ignored) | | T8 | Canceler `cargo test` (no `--ignored`) | Integration tests skipped | | T9 | `docs/testing.md` | No “run `./scripts/e2e-test.sh`” as current CI | ## Test plan (attack, hack, and abuse) Non-exploitative. CI fixtures only. These catch fail-open test gating, not a drain recipe. | # | Vector | Expect | | --- | --- | --- | | A1 | Skip `fraud_detection` because env is empty | Job red | | A2 | Skip `canceler_solana_source_fraud` because `SOLANA_ENABLED` unset **if classified security-critical** | Job red (or the test is not in the default required set — then it must not be counted as coverage) | | A3 | Reintroduce `chmod +x ./scripts/e2e-test.sh` | Grep / CI check fails | | A4 | Woodpecker path-filter back to the three hash-verify files | Review fail; unit step must be unfiltered `test:unit` | | A5 | Playwright smoke without `E2E_UI_ONLY` pulling compose on Woodpecker | Out of scope / forbidden in this ticket | | A6 | `cargo test` in canceler job without `--ignored` as the only canceler command | Fail AC6 | | A7 | Comment in YAML “NO SKIPPING” while skip is still success | Code path A1 still fails; comment is not the control | ## Verification criteria - Unit tests around `TestSuite` exit policy (T1–T3) in `packages/e2e`. - Workflow grep for `e2e-test.sh`. - Woodpecker YAML: `npm run test:unit` with no extra test-file argv; Playwright smoke env `E2E_UI_ONLY=1`. - Canceler job log shows ignored tests **executed** when infra is up (`running 1 test` for an Anvil connectivity case), not `ignored`. - Do not verify by skipping a live fraud case on production RPC. ## Out of scope - #172 operator live execute / Woodpecker **enablement**. - #186 Solana CLI pipe-to-shell. - #185 Compose `:latest` digest pins (including `e2e.yml` `localterra-cl8y:latest`). - Moving Foundry / Anchor / full `cl8y-e2e full` onto Woodpecker. - Full Playwright transfer matrix (`transfer-*-to-*.spec.ts`) on Woodpecker. - Un-ignoring operator `packages/operator/tests/integration_test.rs` (sibling ignore; only if the same `e2e.yml` step is a one-line add — not required to close TEST-1). - Changing canceler/operator runtime logic. ## First-pass model recommendation Recommendation: grok-high Rationale: Cross-cutting CI policy across `packages/e2e` (exit policy), `.github/workflows/e2e.yml` (dead script + canceler `--ignored`), `.woodpecker.yaml` (frontend unit + Playwright smoke), canceler integration attributes, and `docs/testing.md`. More than three production files and more than one subsystem. Woodpecker is the required merge check — a wrong skip allowlist or a Playwright step that pulls compose will either hide security tests again or break every PR. File count does not make this Composer-eligible. Verify with T1–T3 fixtures, workflow greps, and a Woodpecker PR that fails on a deliberate security skip / omitted unit file — not by asserting a green pipeline that skipped the suite.

PlasticDigits added the

bug

agent:implement

labels

2026-09-12 12:58:56 +00:00

PlasticDigits commented

2026-09-12 12:59:05 +00:00

Author

Owner

cl8y-agent-control: queued implement job 8ee641fd-213f-42e9-89d1-eef32a0a9139 (not executed; no Hetzner VM).

cl8y-agent-control: queued `implement` job `8ee641fd-213f-42e9-89d1-eef32a0a9139` (not executed; no Hetzner VM).

PlasticDigits referenced this issue