This repository was archived by the owner on May 4, 2026. It is now read-only.
TCK-00573: Job unit sandbox hardening: NoNewPrivileges, PrivateTmp, Protect*, Restrict*, safe defaults without breaking builds#711
Merged
Conversation
Collaborator
Author
# apm2-review-verdict:v1
schema: apm2.review.verdict.v1
pr: 711
sha: 5d12adbceef61a24b53e2671a27221c2bf5788f2
updated_at: 2026-02-17T04:15:16Z
dimensions:
code-quality:
decision: deny
reason: 'FAIL: 3 blocker, 1 major findings. Includes security regression (variable-time hash comparison) and breaking data changes (unreadable stored receipts).'
set_by: ubuntu
set_at: 2026-02-17T04:14:34Z
security:
decision: deny
reason: 'BLOCKER: Silent Revert of Security Features (TCK-00573) and deceptive PR scope. MAJOR: Unjustified migration of security gates to Shell scripts and breaking CLI changes.'
set_by: ubuntu
set_at: 2026-02-17T04:15:16Z
findings:
- finding_id: f-711-code_quality-1771301638871907-0
type: code-quality
severity: BLOCKER
summary: Variable-time comparison of cryptographic digests
risk: Timing side-channel leaks digest content during integrity verification.
impact: Attacker can potentially bypass receipt integrity verification via timing analysis of the BLAKE3 digest comparison.
location: crates/apm2-core/src/fac/receipt_index.rs:730
body: 'The PR reverts the use of constant-time comparison (ct_eq) in verify_receipt_integrity and replaces it with the variable-time ''=='' operator. This violates invariant INV-PC-001 and security requirement RSK-1901. REQUIRED ACTION: Restore use of subtle::ConstantTimeEq for comparing digests.'
evidence_digest: 66e6a42f454bb631f9cc984301e9a8cfc0cac74ba9d759050719aa9b590c5b17
evidence_pointer: none
timestamp: 2026-02-17T04:13:58Z
- finding_id: f-711-code_quality-1771301643315756-0
type: code-quality
severity: BLOCKER
summary: Breaking change for stored receipts (Deserialization Failure)
risk: Existing receipts containing 'sandbox_hardening_hash' will fail to deserialize.
impact: Complete loss of readability and integrity verification for all receipts generated during the TCK-00573 deployment window. The 'deny_unknown_fields' attribute on FacJobReceiptV1 ensures hard failure upon encountering the removed field.
location: crates/apm2-core/src/fac/receipt.rs:713
body: 'The removal of the ''sandbox_hardening_hash'' field from the ''FacJobReceiptV1'' struct, combined with the ''#[serde(deny_unknown_fields)]'' attribute, makes the system unable to load any previously persisted receipts that include this field. REQUIRED ACTION: If a revert is intended, the field must be retained as an ''Option<String>'' (possibly deprecated) to preserve backward compatibility with existing data, or a migration/transition plan must be documented.'
evidence_digest: 2b28d332c3cc0e00b87cb75b52819796490c5281b3c14466f260a0f226364a99
evidence_pointer: none
timestamp: 2026-02-17T04:14:03Z
- finding_id: f-711-code_quality-1771301647588389-0
type: code-quality
severity: BLOCKER
summary: 'Missing requirement: Add systemd hardening directives'
risk: The PR directly violates the primary scope of the linked ticket TCK-00573.
impact: The security feature (Sandbox Hardening) is completely removed rather than added, leaving job units with a wider-than-necessary attack surface. Documentation and implementation are fundamentally misaligned.
location: TCK-00573.yaml
body: 'The ticket TCK-00573 explicitly mandates adding 10 specific systemd hardening directives to job units. This PR does the exact opposite by removing the implementation of these directives. Even if this is intended as a stabilization step for TCK-00613, it leaves TCK-00573 in a failed state. REQUIRED ACTION: Either fulfill the original requirement or explicitly pivot the ticket to document the removal justification.'
evidence_digest: 5bf75e7e06d0e52d0565e2b1b356de13c240ac6f403ff228f27cf9f9b7a8f864
evidence_pointer: none
timestamp: 2026-02-17T04:14:07Z
- finding_id: f-711-code_quality-1771301651543224-0
type: code-quality
severity: MAJOR
summary: Inconsistent Ticket Documentation
risk: Requirements documentation (YAML) conflicts with implementation.
impact: Future audits or implementations relying on the ticket will find conflicting information about whether systemd hardening is required.
location: TCK-00573.yaml
body: 'The PR updates TCK-00573.yaml to remove GC stabilization lines but leaves ''Add systemd hardening directives'' in the in_scope section. This creates an internal contradiction within the ticket and between the ticket and the code. REQUIRED ACTION: Synchronize the ticket YAML with the actual implemented state.'
evidence_digest: b75d016306654d1b04972513fd34520780a06eedb1bf1ceb7e52d8227ffa6a84
evidence_pointer: none
timestamp: 2026-02-17T04:14:11Z
- finding_id: f-711-code_quality-1771301656005197-0
type: code-quality
severity: MINOR
summary: Broken V1 Canonicalization Stability
risk: Loss of integrity verification for historical receipts.
impact: Metadata (sandbox_hardening_hash) is lost from the canonical preimage, preventing content-hash verification for any receipts that previously included this field.
location: crates/apm2-core/src/fac/receipt.rs:850
body: 'The removal of the optional ''sandbox_hardening_hash'' field from the V1 ''canonical_bytes'' method breaks bit-for-bit compatibility for any receipts generated with that field. While V1 is intended to be stable, this revert effectively orphans the metadata of receipts created during the deployment window. REQUIRED ACTION: Consider if historical verification is required; if so, retain the field in canonicalization logic.'
evidence_digest: 5ce9080dae06006e613b4d57b65f425ed7acc20c724c298d73d280b8c87e6cc7
evidence_pointer: none
timestamp: 2026-02-17T04:14:16Z
- finding_id: f-711-security-1771301683653603-0
type: security
severity: BLOCKER
summary: Silent Revert of Security Features (TCK-00573) and Massive Scope Creep
risk: 'The PR reverts major security controls (Sandbox Hardening TCK-00573) and reopens closed tickets (TCK-00532, TCK-00534, etc.) without disclosure in the title or description. The title ''test(fac): accept fail-closed FAC policy parse error'' is deceptively narrow, hiding a systemic rollback of security features.'
impact: Loss of systemd sandbox hardening (PrivateTmp, NoNewPrivileges, etc.) for FAC jobs. Inconsistency between code state and documentation/tickets. Disruption of audit trails due to removal of hardening hashes from receipts.
location: crates/apm2-core/src/fac/systemd_properties.rs
body: The PR removes and its enforcement logic from , , and , effectively reverting TCK-00573. It also reopens multiple closed tickets in . This appears to be a massive revert or refactor disguised as a minor test fix.\n\nRemediation:\n1. Update the PR title and description to accurately reflect the scope (e.g., 'Revert TCK-00573 and migrate gates to shell').\n2. Justify the removal of Sandbox Hardening (TCK-00573) which was a documented security control.\n3. Split the refactor (migration to shell scripts) from the logic revert if possible.
evidence_digest: 70b9098bc317ba213c7b0f43cee9ba841c8f7b45ebc87b0b7f24bbdde9bf54ca
evidence_pointer: none
timestamp: 2026-02-17T04:14:43Z
- finding_id: f-711-security-1771301696432933-0
type: security
severity: BLOCKER
summary: 'CORRECTION: Silent Revert of Security Features (TCK-00573)'
risk: 'The PR reverts major security controls (Sandbox Hardening TCK-00573) and reopens closed tickets (TCK-00532, TCK-00534, etc.) without disclosure in the title or description. The title ''test(fac): accept fail-closed FAC policy parse error'' is deceptively narrow, hiding a systemic rollback of security features.'
impact: Loss of systemd sandbox hardening (PrivateTmp, NoNewPrivileges, etc.) for FAC jobs. Inconsistency between code state and documentation/tickets. Disruption of audit trails due to removal of hardening hashes from receipts.
location: crates/apm2-core/src/fac/systemd_properties.rs
body: The PR removes `SandboxHardeningProfile` and its enforcement logic from `systemd_properties.rs`, `policy.rs`, and `receipt.rs`, effectively reverting TCK-00573. It also reopens multiple closed tickets in `documents/work/tickets/`. This appears to be a massive revert or refactor disguised as a minor test fix.\n\nRemediation:\n1. Update the PR title and description to accurately reflect the scope (e.g., 'Revert TCK-00573 and migrate gates to shell').\n2. Justify the removal of Sandbox Hardening (TCK-00573) which was a documented security control.\n3. Split the refactor (migration to shell scripts) from the logic revert if possible.
evidence_digest: baafd55f4e7593fe7781018e0ec9a35ef692fb3d7991510bdda53ed1da87bf70
evidence_pointer: none
timestamp: 2026-02-17T04:14:56Z
- finding_id: f-711-security-1771301707153713-0
type: security
severity: MAJOR
summary: Migration of Security Gates from Rust to Shell Scripts
risk: Security-critical logic (test safety guard, preflight authorization, credential hardening) has been migrated from compiled, type-safe Rust code in `apm2-cli` to Bash scripts in `scripts/ci/`. Shell scripts are historically more prone to quoting errors, TOCTOU race conditions, and platform dependency issues (reliance on `rg`, `jq`, etc.).
impact: Reduced assurance in CI guardrails. Increased maintenance burden and fragility.
location: scripts/ci/
body: The PR deletes `fac_gate_checks.rs`, `fac_preflight.rs` and replaces them with `test_safety_guard.sh`, `fac_preflight_authorization.sh`, etc. While the scripts appear to port the logic, the move away from the primary compiled artifact (`apm2`) is a regression in engineering rigor unless strictly required for bootstrapping.
evidence_digest: caa5a839c16d526ae320655da199e98ca627222da16cfc7c7720789892a5f6e6
evidence_pointer: none
timestamp: 2026-02-17T04:15:07Z
- finding_id: f-711-security-1771301707279659-0
type: security
severity: MAJOR
summary: 'Breaking Change: Removal of ''fac preflight'' Command'
risk: The `apm2 fac preflight` command has been removed. Any external CI workflows or tools relying on this command for security checks will fail or bypass checks if they are not atomically updated.
impact: Potential bypass of preflight checks if CI configuration drifts from the new script-based path.
location: crates/apm2-cli/src/commands/fac.rs
body: The PR removes the `Preflight` subcommand from `fac.rs`. While `.github/workflows/forge-admission-cycle.yml` is updated in this PR, other consumers may break.
evidence_digest: 46d17d618e64ccc27be4565b8cbc1bc0c449b87832e4a4cb9bb4609986cc085c
evidence_pointer: none
timestamp: 2026-02-17T04:15:07Z |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
FAC Gate Status