[diberry]

Closes bradygaster/squad#888

What

20 structural test assertions verifying security and hardening properties of all 11 shipped workflow templates in .squad-templates/workflows/.

Test Results

• 58 passing — current invariants enforced (permissions blocks, no pull_request_target, no secrets:inherit, concurrency group sanity, SHA gating)
• 55 todo — hardening roadmap (timeout-minutes, SHA pinning, concurrency blocks, cancel-in-progress policies, npm cache, bot guards, persist-credentials, id-token scoping, paths-filter, COPILOT_ASSIGN_TOKEN pattern)
• 0 failures — clean run

Run

node --test test/template-ci-hardening.test.cjs

Files

• test/template-ci-hardening.test.cjs — main test file (20 assertion categories)
• test/fixtures/known-good-shas.json — SHA reference for consistency checks

Working as FIDO (Quality Owner)

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

[github-actions]

🟢 Impact Analysis — PR #915

Risk tier: 🟢 LOW

📊 Summary

Metric	Count
Files changed	2
Files added	2
Files modified	0
Files deleted	0
Modules touched	1

🎯 Risk Factors

• 2 files changed (≤5 → LOW)
• 1 module(s) touched (≤1 → LOW)

📦 Modules Affected

tests (2 files)

• test/fixtures/known-good-shas.json
• test/template-ci-hardening.test.cjs

---

This report is generated automatically for every PR. See #733 for details.

[diberry]

🔍 Squad Review — Kaylee (Engineering)

#	Check	Status	Notes
1	Changelog entry	✅	skip-changelog label applied
2	Squashed to 1 commit	✅	1 commit
3	CI green	✅	All 11 checks passed/skipped
4	Copilot comments resolved	✅	0 unresolved threads
5	No .squad/ files	✅	Clean
6	No unrelated files	✅	2 files — test + fixture, both in scope
7	Tests for changes	✅	Test-only PR (20 structural assertions)
8	Not a duplicate/reversal	✅	Complements #887 (templates) — this is tests

Verdict: ✅ Ready to merge

---

Review by Squad AI team (Kaylee — Engineering) · requested by Dina Berry

[diberry]

🏗️ Flight — Architecture Review

Verdict: ⚠️ COMMENT

Findings

1. Strong structural invariant framework. 20 assertions covering timeout-minutes, SHA pinning, concurrency, permissions, pull_request_target prohibition, secrets: inherit prohibition, and id-token scoping. The passing-vs-todo split is honest — it enforces what's true today and roadmaps the rest.
2. Uses ode:test (CJS) instead of vitest. This is a deliberate divergence from the project's test framework. Architecturally, this makes sense for template tests that verify static YAML properties — they don't need vitest's mocking or SDK imports. But it creates a second test runner path. Question: Is this covered by pm test, or does it need a separate CI step? If it requires ode --test, the CI pipeline needs updating.
3. known-good-shas.json fixture is forward-looking. The SHA consistency check (A13) uses this fixture to verify all templates reference the same SHA for a given action. Currently templates use @v tags, so the fixture is dormant — it activates automatically when SHA pinning lands. Good design.
4. Template directory scope is correct. Tests read from .squad-templates/workflows/ (the source-of-truth for shipped templates). Does not test .github/workflows/ (repo-specific) — correct boundary.

Module Boundary Check

• No boundary violations. Test-only PR, no package code touched. The CJS format avoids importing from squad-sdk/cli packages.

Recommendation

Confirm the test runner integration: does pm test pick up .test.cjs files via the vitest config, or does this need a separate ode --test step in CI? If the latter, add it to squad-ci.yml so these assertions actually run on PRs. The test framework is sound but useless if it doesn't execute in CI.

[diberry]

🔒 RETRO — Security Review

Verdict: ✅ APPROVE

Security Findings

• Test-only PR using node:test and node:assert/strict built-ins. No production code changes.
• known-good-shas.json provides a SHA reference table for GitHub Actions pinning — security positive (enables future SHA-pinning enforcement).
• Tests validate security properties: pull_request_target prohibition (A16), secrets: inherit prohibition (A18), permissions blocks (A15), SHA format (A11), concurrency groups (A20).
• The test uses test.todo() for not-yet-hardened assertions — correct approach for a phased roadmap.

Credential / PII Check

• No secrets or PII. SHA values in the fixture are public commit hashes for well-known GitHub Actions.

Workflow Permissions

• N/A — no workflow changes. This is a test file only.

Recommendation

Clean merge. This test suite is a security multiplier — it codifies the workflow hardening invariants. Strong APPROVE.

[diberry]

🧪 FIDO — Quality Review

Verdict: ✅ APPROVE

Test Coverage

• 20 structural assertions covering: permissions blocks (A15), no pull_request_target (A16), concurrency groups (A3, A5, A20), SHA format validation (A11), SHA version comments (A12), SHA consistency (A13), no secrets: inherit (A18), no workflow-level id-token: write (A17), no npm cache without setup-node (A7), cache: npm on setup-node (A6).
• 55 test.todo() items define the hardening roadmap (SHA pinning, timeout-minutes, bot guards, etc.). Good use of todo to track future work without false passes. ✓
• known-good-shas.json fixture enables SHA consistency enforcement across templates. ✓

Edge Cases / Risks

• Uses node:test, not vitest. The project standard is vitest, but this file uses Node.js built-in test runner. This means npm test (vitest) won't run these tests. They need a separate node --test invocation. Verify this is wired into CI. If not, these tests exist but never run.
• Template discovery is dynamic (readdirSync). Good — survives template additions without test updates. ✓
• parseWorkflowStructure is basic. It splits on 2-space indentation which could break on YAML comments or multi-line strings. Acceptable for current templates, but fragile if templates get more complex.

CI Impact

• New test file. Must confirm it's included in a CI job. If it's only runnable via node --test and CI only runs npm test (vitest), these 58 passing tests are invisible to CI.

Recommendation

Approve with one action item: verify node --test test/template-ci-hardening.test.cjs is executed in CI. If not, add it as a CI step. Tests that don't run in CI are decorative.