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)#710

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

TCK-00613: FAC gates architecture alignment + security hardening (throughput-profile model)#710
Anveio merged 2 commits 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
  - short_sha: 7a3a91f9
    message: Harden no-wait worker bootstrap and symlink integrity hashing
  - short_sha: '34447208'
    message: Align gate_checks formatting with FAC rustfmt gate

FAC Gate Status

# apm2-gate-status:v2
sha: 34447208c59d2acae6f4872d1fb29cb0a01420a9
short_sha: 34447208
timestamp: '2026-02-17T03:23:44Z'
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: 24
  - name: 'test_safety_guard'
    status: PASS
    duration_secs: 2
  - name: 'test'
    status: PASS
    duration_secs: 77
  - 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 03:23
@Anveio Anveio merged commit cba6a94 into main Feb 17, 2026
2 of 3 checks passed
@Anveio Anveio deleted the ticket/RFC-0019/TCK-00613 branch February 17, 2026 03:24
@Anveio

Anveio commented Feb 17, 2026

Copy link
Copy Markdown
Collaborator Author
# apm2-review-verdict:v1
schema: apm2.review.verdict.v1
pr: 710
sha: 34447208c59d2acae6f4872d1fb29cb0a01420a9
updated_at: 2026-02-17T03:39:23Z
dimensions:
  code-quality:
    decision: deny
    reason: 'FAIL: 1 blocker, 0 major findings'
    set_by: ubuntu
    set_at: 2026-02-17T03:30:01Z
  security:
    decision: approve
    reason: 'PASS: no blocker or major findings'
    set_by: ubuntu
    set_at: 2026-02-17T03:39:23Z
findings:
- finding_id: f-710-code_quality-1771298995186655-0
  type: code-quality
  severity: BLOCKER
  summary: Inverted wait/no-wait worker bootstrap condition prevents wait-mode bootstrap
  risk: The CLI condition inversion makes default wait mode skip worker bootstrap while no-wait mode bootstraps a worker process, reversing intended foreground vs background semantics.
  impact: In default wait mode, review work may never be processed while the command appears to run successfully; no-wait can unexpectedly start bootstrap work.
  location: crates/apm2-cli/src/commands/fac_review/gates.rs:256
  body: 'The implementation uses if wait { false } else { ensure_non_wait_worker_bootstrap(...) }, while invocation sets wait = args.wait && !args.no_wait. This inverts intent: wait-enabled calls do not run the no-wait bootstrap path and no-wait calls do. Align the branch semantics with wait/no-wait intent and add regression coverage for both combinations.'
  evidence_digest: d56cf1604f621f9cd33dbe59ac54ae15ed6f7dd5fea1acf811d840514313827e
  evidence_pointer: none
  timestamp: 2026-02-17T03:29:55Z

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