Skip to content
This repository was archived by the owner on May 4, 2026. It is now read-only.

TCK-00613: FAC gates architecture alignment + security hardening (throughput-profile model)#709

Merged
Anveio merged 1 commit into
mainfrom
ticket/RFC-0019/TCK-00613
Feb 17, 2026
Merged

TCK-00613: FAC gates architecture alignment + security hardening (throughput-profile model)#709
Anveio merged 1 commit into
mainfrom
ticket/RFC-0019/TCK-00613

Conversation

@Anveio

@Anveio Anveio commented Feb 17, 2026

Copy link
Copy Markdown
Collaborator
ticket_meta:
  schema_version: 2026-01-29
  template_version: 2026-01-29
  ticket:
    id: TCK-00613
    title: FAC gates architecture alignment + security hardening (throughput-profile model)
    status: OPEN
  binds:
    prd_id: PRD-PLACEHOLDER
    rfc_id: RFC-0019
    requirements: []
    evidence_artifacts: []
  custody:
    agent_roles:
    - AGENT_IMPLEMENTER
    responsibility_domains:
    - DOMAIN_RUNTIME
    - DOMAIN_SECURITY
    - DOMAIN_DOCUMENTATION
  dependencies:
    tickets:
    - ticket_id: TCK-00612
      reason: Defines the throughput-profile model (`throughput`/`balanced`/`conservative`) and host-aware parallelism contract used by fac gates.
    - ticket_id: TCK-00605
      reason: Defines bounded test execution constraints and containment policy used by FAC gates.
    - ticket_id: TCK-00563
      reason: Canonicalizer tuple pinning and receipt integrity remain required invariants in worker/broker admission flows.
  root_cause_analysis:
    summary: |
      The prior revision of this ticket mixed two incompatible directions:
      - A new trait/registry gate engine with gate-set `--profile`.
      - The already-adopted command-centric FAC gates runtime with throughput
        profiles and typed gate checks.

      In parallel, PR review surfaced concrete security and correctness gaps:
      - Signing key TOCTOU and duplicated key-handling paths.
      - Unbounded file reads in gates/preflight paths.
      - Worker workspace-root trust boundary ambiguity.
      - Process-global CWD mutation in worker execution path.
      - workflow_dispatch authorization trust override via env var.
      - test_safety_guard misses on top-level `src/*.rs` and unbounded traversal.

      This ticket update formalizes the architecture actually in use and binds
      DoD to hardened, fail-closed behavior across FAC gates and preflight.
  scope:
    in_scope:
    - 'Architecture contract: command-centric gates execution remains authoritative (`fac_review/gates.rs` orchestration + typed checks in `fac_review/gate_checks.rs`).'
    - 'Gate selection semantics: keep `--quick` as gate-set selector (fast subset) and full mode as default behavior.'
    - 'Throughput semantics: keep `--gate-profile` (`throughput`/`balanced`/`conservative`) for resource/concurrency policy, independent of gate-set selection.'
    - Introduce shared hardened signer-key utility (`fac_key_material`) and remove duplicated key creation/loading logic across queue submit, worker, and broker paths.
    - Introduce shared fail-closed bounded-read utility (`fac_secure_io`) and replace raw unbounded reads in FAC review gate checks, FAC preflight, FAC broker, queue submit, and worker-adjacent read sites.
    - Enforce fail-closed handling for oversized/unreadable inputs in security-sensitive paths (no skip-on-error behavior for authorization/gate enforcement inputs).
    - Remove process-global CWD mutation from worker execution path; pass explicit workspace root through the gates call chain.
    - 'Define and enforce workspace-root admission policy in worker gates execution: canonical dir, git toplevel equality, repo identity binding, and explicit deny for FAC-internal roots.'
    - 'Strengthen preflight trust binding: no env override for actor permission, validated repository/actor normalization, bounded event/policy/PR JSON parsing, and explicit deny on permission lookup failure.'
    - 'Harden test_safety_guard determinism and coverage: tracked-file-first targeting, top-level `src/lib.rs` and `src/main.rs` coverage under `#[cfg(test)]`, and bounded fail-closed file reads.'
    - Add regression tests for key creation race, workflow_dispatch permission checks, oversized payload denial, workspace-root policy denial, and top-level Rust test-safety detection.
    out_of_scope:
    - Mandating a `gates_engine/` trait/registry abstraction (`Gate`, `GateRegistry`, `GateRunner`) in this ticket.
    - Replacing current gate-set semantics with a new `--profile <gate-set>` CLI contract in this ticket.
    - Repository-wide deletion of all shell scripts outside the FAC gates/preflight hardening scope.
    - Backward-compatibility shims for superseded ticket language.
    - Global scheduler redesign or distributed queue architecture changes.
  plan:
    steps:
    - id: STEP_01
      title: Codify architecture and CLI contract
      detail: |
        Lock the canonical model for this ticket:
        - `fac_review/gates.rs` orchestrates execution.
        - `--quick` controls gate-set shape.
        - `--gate-profile` controls throughput/resource policy only.
    - id: STEP_02
      title: Centralize hardened key material handling
      detail: |
        Add shared signer utility for secure create/load semantics:
        - create with owner-only mode at open time
        - nofollow safeguards
        - fsync of file and parent dir
        - race-safe concurrent creator convergence
        Replace all duplicate key handling in queue submit/worker/broker.
    - id: STEP_03
      title: Centralize bounded fail-closed file reads
      detail: |
        Add shared bounded I/O helper and migrate targeted paths off raw
        `fs::read` / `fs::read_to_string` for security-sensitive inputs.
    - id: STEP_04
      title: Harden worker execution boundary
      detail: |
        Remove ambient CWD dependency and enforce explicit workspace-root
        admission checks (canonicalize + git root + repo bind + FAC-internal
        deny rules).
    - id: STEP_05
      title: Harden preflight trust binding
      detail: |
        Remove env-based actor-permission override from authorization path,
        enforce repository/actor normalization, and deny on oversized/invalid
        preflight payloads or permission lookup failures.
    - id: STEP_06
      title: Fix gate correctness regressions
      detail: |
        Ensure test_safety_guard covers top-level `src/*.rs` test regions and
        uses bounded deterministic file targeting/read behavior.
    - id: STEP_07
      title: Regression suite and verification
      detail: |
        Run targeted security/correctness regressions and strict quality gates:
        - `cargo fmt --all`
        - `cargo clippy -p apm2-cli --all-targets --all-features -- -D warnings`
        - `cargo test -p apm2-cli`
  definition_of_done:
    evidence_ids: []
    criteria:
    - 'Ticket language is internally consistent with implemented architecture: command-centric gates orchestration + throughput profiles.'
    - Signer key lifecycle is centralized; queue submit, worker, and broker use shared hardened key material helpers.
    - No production path creates signing keys via write-then-chmod TOCTOU sequence.
    - Bounded read helper is shared and used across targeted FAC gates/preflight/broker/queue paths; oversized inputs are denied fail-closed.
    - Worker gates execution path does not mutate process-global CWD.
    - Worker workspace_root admission denies FAC-internal roots and repo-identity mismatches; git toplevel binding is enforced.
    - workflow_dispatch authorization does not trust env permission overrides and denies on lookup failure or insufficient permission.
    - Preflight event/policy payload reads are bounded and oversized payloads fail closed.
    - test_safety_guard evaluates top-level `src/lib.rs` and `src/main.rs` test regions and fails closed on oversized targets.
    - Regression tests exist for signer concurrency, workflow_dispatch permission checks, oversized payload denial, and workspace-root policy denial.
    - '`cargo fmt --all` passes.'
    - '`cargo clippy -p apm2-cli --all-targets --all-features -- -D warnings` passes.'
    - '`cargo test -p apm2-cli` passes.'
  notes:
    context: |
      This amendment aligns TCK-00613 with the architecture currently used by
      FAC gates in this repository branch:
      - command-centric orchestration in `fac_review/gates.rs`
      - typed gate checks and evidence checks under `fac_review/*`
      - throughput profile model from TCK-00612

      It intentionally removes conflicting requirements that assumed this ticket
      was also the migration vehicle for a trait/registry gate engine and
      gate-set profile CLI redesign.
    amendments:
    - amendment_id: AMD-2026-02-17-ARCH-REALIGNMENT
      summary: Replace conflicting gate-engine/profile requirements with branch-aligned architecture and hardening scope.
      supersedes:
      - Requirement that TCK-00613 must introduce `gates_engine/` with `Gate` trait, `GateRegistry`, and `GateRunner`.
      - Requirement that TCK-00613 must replace `--quick` with gate-set `--profile` semantics.
      - Requirement that TCK-00613 must be the single vehicle for repository-wide script eradication.
      replacement:
      - TCK-00613 defines and hardens the existing command-centric FAC gates model with explicit fail-closed security guarantees.
      - Gate-set profile CLI redesign and trait/registry abstraction, if still desired, are follow-on architecture work and not blockers for this ticket.
    security: default-deny, least privilege, fail-closed
fac_push_metadata:
  commit_history:
  - short_sha: a1243893
    message: TCK-00613 add unified gate engine ticket, close 5 merged TCKs
  - short_sha: be3acbb2
    message: cut over FAC CI preflight/gates to Rust and remove shell scripts
  - short_sha: 667de9a9
    message: stabilize FAC test home isolation and env-mutating test locks
  - short_sha: 8fb26f32
    message: Harden FAC admission binding and stabilize gate execution
  - short_sha: 04a10c79
    message: Update TCK-00613 with stabilization fixes and FAC doctor findings
  - short_sha: a302f44d
    message: Fix FAC doctor registry-integrity recovery and convergence
  - short_sha: d110dbb8
    message: Harden FAC gates/preflight security paths and realign TCK-00613 architecture
  - short_sha: 8942f3c8
    message: Harden FAC preflight trust binding and gate/workspace admission
  - short_sha: 847f5226
    message: Make fac gates wait-by-default and self-process queue
  - short_sha: a9a40039
    message: Harden tracked-path decoding and key parent creation

FAC Gate Status

# apm2-gate-status:v2
sha: a9a40039f98cc3056040cc02207b069ed8f9eba1
short_sha: a9a40039
timestamp: '2026-02-17T02:43:36Z'
all_passed: true
gates:
  - name: 'merge_conflict_main'
    status: PASS
    duration_secs: 0
  - name: 'rustfmt'
    status: PASS
    duration_secs: 3
  - name: 'clippy'
    status: PASS
    duration_secs: 43
  - name: 'doc'
    status: PASS
    duration_secs: 22
  - name: 'test_safety_guard'
    status: PASS
    duration_secs: 2
  - name: 'test'
    status: PASS
    duration_secs: 78
  - name: 'workspace_integrity'
    status: PASS
    duration_secs: 0
  - name: 'review_artifact_lint'
    status: PASS
    duration_secs: 0

@Anveio Anveio enabled auto-merge February 17, 2026 02:43
@Anveio Anveio merged commit dad68b1 into main Feb 17, 2026
2 of 3 checks passed
@Anveio Anveio deleted the ticket/RFC-0019/TCK-00613 branch February 17, 2026 02:44
@Anveio

Anveio commented Feb 17, 2026

Copy link
Copy Markdown
Collaborator Author
# apm2-review-verdict:v1
schema: apm2.review.verdict.v1
pr: 709
sha: a9a40039f98cc3056040cc02207b069ed8f9eba1
updated_at: 2026-02-17T03:03:35Z
dimensions:
  code-quality:
    decision: deny
    reason: 'FAIL: 0 blocker, 1 major findings'
    set_by: ubuntu
    set_at: 2026-02-17T02:55:12Z
  security:
    decision: deny
    reason: 'FAIL: 0 blocker, 1 major findings'
    set_by: ubuntu
    set_at: 2026-02-17T03:03:35Z
findings:
- finding_id: f-709-code_quality-1771296903631584-0
  type: code-quality
  severity: MAJOR
  summary: --no-wait fails when no FAC worker is alive
  risk: --no-wait cannot be used to enqueue deferred gates because run_gates_via_worker rejects when no worker heartbeat exists.
  impact: Any async FAC gate enqueue flow can fail spuriously when worker is not currently running, preventing job staging and breaking expected queue-only usage.
  location: crates/apm2-cli/src/commands/fac_review/gates.rs:256
  body: In run_gates_via_worker, --no-wait (wait=false) is rejected whenever no worker heartbeat is present. This closes the asynchronous dispatch path even though the validated job can still be safely persisted. It regresses the documented --no-wait behavior (return immediately after enqueue) and prevents queue usage in worker-less windows. Keep enqueue behavior for --no-wait unconditional, and only use heartbeat checks for inline/waiting execution paths.
  evidence_digest: f5058f641d29e718530fb23e7a15608a91ce00f9750b5fbef7abca4b9e33f42f
  evidence_pointer: none
  timestamp: 2026-02-17T02:55:03Z
- finding_id: f-709-security-1771297408868286-0
  type: security
  severity: MAJOR
  summary: Workspace integrity snapshots hash symlink target paths, not target contents
  risk: An attacker can add or modify tracked symlink files so workspace integrity manifests never reflect changes to the linked target. This can bypass the workspace integrity gate by keeping snapshot hashes stable while executed content changes on disk.
  impact: Mutated target files can carry unreviewed code or artifacts without detection, undermining hash-based workspace integrity checks and allowing untrusted changes to proceed through later FAC gates or tests.
  location: crates/apm2-cli/src/commands/fac_review/gate_checks.rs:hash_file
  body: hash_file hashes read_link() path bytes for symlink-tracked files instead of reading target contents. Gate snapshot/verify compare path hashes, so content mutations through a tracked symlink are invisible. Bind integrity checks to canonicalized target bytes or deny symlink indirections for tracked files so target-file content and mode changes are always included.
  evidence_digest: df7963ab44af881190ee65fd4fab3c958ff4d07c57351693961af29faa710833
  evidence_pointer: none
  timestamp: 2026-02-17T03:03:28Z

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant