fix(signer): sample authorization time after acquiring transaction lock

docs/design-decisions/SECURITY-INCREMENT.md

@@ -0,0 +1,145 @@
+# Signer Transaction-Time Freshness
+
+## Problem
+
+An authorized request could wait for the signer lock across its token or
+evaluation expiry, then be accepted using time sampled before that wait.
+The bounded reproduction is documented in
+[THREAT-BOUNDARY-AUDIT.md](THREAT-BOUNDARY-AUDIT.md).
+
+## Invariant
+
+Admission and finalization must sample authorization time after acquiring the
+shared transaction lock. Expired OIDC is refused before admission or publication;
+an expired evaluation with valid execution identity is refused after consuming
+its finalization JTI, as required by the existing replay contract.
+
+## Design decision
+
+Move the existing `now = self._now()` statement inside `store.transaction()` in
+the two service entrypoints. This changes two statement locations and introduces
+no dependency, new error code, schema, authority source, or policy identity.
+Ticket issuance time and TTL now begin at transaction entry. An existing
+idempotent ticket retains its original frozen timestamps.
+
+## Threat and failure model
+
+The adversarial schedule is: request arrives with valid identity, waits behind
+another lock holder, expires, then acquires the lock. Both new admission and
+idempotent admission must refuse an expired identity. Finalization must refuse
+expired identity or evaluation without reserving a slot or calling the publisher.
+The local reproduction uses synthetic claims and a controlled clock; it assumes
+neither stolen real credentials nor control over production server time.
+
+## Rejected alternatives
+
+1. Extend token/evaluation lifetimes or add queue-time grace. Rejected because
+   this would weaken the frozen lifetime contract instead of checking it correctly.
+2. Replace the shared lock with a new transaction/outbox architecture. Rejected
+   because production persistence is a separate, unauthorized implementation scope.
+3. Change `P_v(H)`, status contexts, or the candidate policy bundle. Rejected
+   because none is responsible for this operational authorization-time defect.
+4. Change equality-at-expiry semantics. Rejected because the existing inclusive
+   contract is separate from the stale-time defect and remains explicitly tested.
+
+## Compatibility
+
+Existing valid requests, frozen registry selection, exact H, canonical payloads,
+publisher source binding, JTI consumption order, and UNKNOWN transitions retain
+their contracts. A request whose identity expires in the queue now fails closed.
+The worker semantic digest contains no new operational timestamp. No workflow,
+protected manifest, baseline, scanner policy, golden, or v1 bundle is changed.
+
+## Validation
+
+Environment: Windows, CPython 3.14.3, Git 2.53.0.windows.3. Tests use synthetic
+fixtures and D-drive temporary storage. Ruff 0.16.7 uses `--isolated` so unrelated
+parent-directory configuration cannot affect this repository's validation.
+
+| Check | Result |
+| --- | --- |
+| Original signer suite before changes | 76 tests, OK. |
+| New regression against original service | 4 methods run; FAILED (failures=5): three OIDC subcases, expired evaluation, and issuance timestamp. Inclusive-expiry control passed. |
+| New regression after correction | 4 methods, OK. |
+| All signer tests after correction | 80 tests, OK. |
+| Full repository suite | 258 tests run: 253 passed, 5 platform skips. |
+| Changed Python Ruff lint and format | Passed; two files already formatted. |
+| Tags, README snapshot, pattern library | Passed. |
+| Markdown/front matter | 157 files passed. |
+| Pre-commit on the four changed files | Passed; out-of-scope hooks skipped. |
+| Scanner integration | Synthetic PASS, FAIL, redaction, and import isolation passed. |
+| Changed-file scanner without baseline | No findings. |
+| Design-document markdownlint | 2 files, 0 errors. |
+| Diff and manual privacy review | Passed; no credentials or machine identifiers added. |
+
+The full-suite skips are four unavailable symlink cases and the POSIX-only
+bootstrap test. They are not signer test skips. The placeholder script excludes
+these design documents and reported no applicable Markdown; it is not a privacy
+certification for these files. Manual review and the changed-file scanner cover
+the new artifacts instead.
+
+Initial tooling attempts found an unusable default Python launcher and a broken
+virtualenv seed cache. Validation used an explicit CPython installation, a fresh
+D-drive validation environment, and a separate virtualenv cache; no repository
+test was weakened. An initial Ruff invocation inherited unrelated parent config;
+the reported checks above use isolated configuration. Scanner false positives
+on long public references and a test identifier were resolved with concise
+references/naming, without baseline or suppression changes.
+
+Exact commands, run from the repository with the validation Python environment
+on PATH (all test fixtures are local):
+
+```powershell
+python -m unittest tests.test_repo_sentinel_signer_time -v
+python -m unittest discover -s tests -p 'test_repo_sentinel_signer*.py'
+python -m unittest discover -s tests -p 'test_*.py'
+python -m ruff check --isolated --select E4,E7,E9,F,B signer/repo_sentinel_signer/service.py tests/test_repo_sentinel_signer_time.py
+python -m ruff format --isolated --check signer/repo_sentinel_signer/service.py tests/test_repo_sentinel_signer_time.py
+python scripts/render_tags_doc.py --check
+python scripts/render_readme_snapshot.py --check
+python scripts/check_pattern_library.py
+python scripts/check_markdown.py
+python scripts/test_repo_sentinel_integration.py
+git diff --check
+```
+
+Pre-commit, the no-baseline changed-file scan, and markdownlint were passed the
+exact four-file list below (markdownlint received only the two Markdown files).
+No standalone build or static type-check configuration applies to this change.
+
+To reproduce the original failure independently, create a disposable worktree
+at base `d35e5a6` on the D drive, copy only
+`tests/test_repo_sentinel_signer_time.py` into it, and run the first command.
+Alternatively, in a disposable copy of this increment, move the two clock
+reads back above their transaction blocks. Do not alter a dirty user checkout.
+
+The tests observe externally meaningful decisions, timestamps, JTI replay,
+slot creation, and publisher calls. They do not merely assert source placement.
+Event synchronization makes the schedule deterministic; a five-second timeout
+only detects a broken test setup and is not an expiry threshold.
+
+## Precise changed-file list
+
+- `signer/repo_sentinel_signer/service.py`: sample time after lock acquisition.
+- `tests/test_repo_sentinel_signer_time.py`: contention regression and controls.
+
+Two documents under `docs/design-decisions/`:
+
+- `THREAT-BOUNDARY-AUDIT.md`: baseline, classifications, assumptions, severity,
+  and evidence boundaries.
+- `SECURITY-INCREMENT.md`: correction and reproduction.
+
+## Rollback and release boundary
+
+Revert this local correction and its regression/documentation together if review
+rejects it; doing so restores the demonstrated stale-time weakness. No remote
+status, deployment, credential, schema, or policy migration is needed.
+
+This increment is local only. A later authorized security-boundary PR must be
+draft and receive next-day review before merge. Production adapters and
+publication-time expiry semantics require their own evidence and authorization.
+
+Recommendation: **ACCEPT** the bounded local correction. A local commit attempt
+was blocked because the configured GPG signer had no available secret key. The
+four-file change remains staged on its dedicated branch. Signing was not
+disabled, no key material was collected, and nothing was pushed or published.

docs/design-decisions/THREAT-BOUNDARY-AUDIT.md

@@ -0,0 +1,103 @@
+# Trusted Authority Boundary Audit
+
+Audit date: 2026-10-15. Scope: one local engineering increment, no deployment.
+
+## Baseline and scope
+
+The audited source is `stacknil/sec-writeups-public` main at
+`d35e5a6`, fetched before creating the
+dedicated local branch `authority-boundary-increment` under `stacknil/`.
+The latest merge is PR #23, the mock signer core. PRs #19, #20, and #21
+already delivered commit authority, the controller, and external policy input.
+Historical notes describing those changes as unmerged are superseded by this
+read-only remote inspection.
+
+Two older main checkouts had unrelated document edits; neither was modified.
+Existing authority branches and an unrelated dirty mutation checkout were
+preserved. PR #22 remains a draft candidate-policy construction at `7c9e513`;
+its candidate bundle is neither production policy nor this increment's base.
+Issue #10 remains open and describes the larger control-plane boundary. Its
+historical prerequisite list is not evidence that merged components are absent.
+
+Reviewed source includes the acquisition/reader contracts, worker/controller
+design records, policy contracts, signer service/store/registry, and signer
+tests. This is a bounded boundary audit, not an exhaustive security certification.
+Research materials outside these repositories were excluded.
+
+## Threat assumptions and attacker capabilities
+
+- PR authors control proposed content, filenames, scripts, and workflow changes.
+  Those bytes must remain data to trusted scanning code.
+- Only a trusted verifier may supply `VerifiedOidcClaims`. No real token,
+  private key, installation credential, or authenticated provider was needed.
+- The selected weakness requires an otherwise authorized execution's request
+  to wait for the shared transaction lock across an expiry boundary. Queue
+  contention may be benign; a caller able to submit concurrent authorized
+  requests could influence it. Production reachability is unproven.
+- The registry, store, process, injected clock, and adapters are trusted in this
+  reference model. A PR author is not assumed able to forge verified claims,
+  alter registry records, or control server time.
+
+## Boundary classification
+
+| Boundary | Classification and evidence | Decision |
+| --- | --- | --- |
+| PR content versus trusted execution | Existing local contracts cover isolated scanner imports, raw object identity, unsupported-entry refusal, inert target code, and exact pull-ref acquisition. | Preserve; no speculative rewrite. |
+| Head H versus test-merge M | Historical canary evidence records M regeneration during merge with fixed B/H. The commit worker defines only `P_v(H)`. | Already rejected design; no new M-only experiment. |
+| Protected name and App source | Historical canary evidence shows an ordinary Actions App name can be spoofed; dedicated expected-App binding rejects the other source's success. Same-name failure can affect availability. | Already characterized; do not change remote contexts or claim availability isolation. |
+| Policy integrity and version binding | Explicit selector registry, external expected digest, frozen epoch identity, exact mirror/coverage controls, and v1 compatibility tests already exist. PR #22 is candidate evidence. | Preserve all bundle and policy bytes. |
+| Replay and authority downgrade | Existing tests cover fresh finalization JTI, malformed-evidence consumption, frozen revision, revocation, same-H slot conflicts, publisher-bound receipts, and UNKNOWN reconciliation. | Preserve these invariants. |
+| Expiry under transaction contention | Confirmed reproducible weakness: both service entrypoints sample time before waiting for the transaction lock. Stale time can authorize after token/evaluation expiry. | **Selected: transaction-time freshness.** |
+| Missing or malformed input | Exact types/keys, independent evidence binding, fixed refusal vocabulary, and no publication for infrastructure refusal have existing tests. | Preserve fail-closed handling. |
+| Real OIDC, durable storage, App publication | Explicitly absent production adapters. Hypothetical deployment risks are not confirmed deployed vulnerabilities. | Outside this increment; no credentials or activation. |
+
+## Selected finding and severity
+
+**Medium, reference implementation only:** stale authorization time across
+transaction lock acquisition. It defeats an explicit lifetime boundary and can
+reach the mock publisher after expiry. It does not establish token forgery,
+arbitrary verdict injection, a head mismatch, or a deployed GitHub bypass.
+
+`SignerService.issue_evaluation()` and `finalize_evaluation()` originally read
+`now` before entering `store.transaction()`. The same snapshot drives OIDC
+validity and, respectively, ticket timestamps or evaluation expiry. A request
+queued at time T can therefore use T after the lock becomes available at T2.
+
+The regression holds the real shared RLock, queues a background service call,
+waits for the transaction-entry signal, advances an injected logical clock,
+then releases the lock. No expiry sleeps, remote writes, scanner execution,
+forged token bytes, or modified registry policy are involved. The original
+source produced five failing assertions; the corrected source passes all four
+test methods, including their admission/finalization subcases.
+
+The precise result and reproduction commands are in
+[SECURITY-INCREMENT.md](SECURITY-INCREMENT.md).
+
+## Remaining uncertainty
+
+This correction establishes freshness at transaction entry. It does not provide
+an atomic wall-clock deadline at remote publication: a future blocking reader,
+publisher, durable transaction, or outbox needs a separately reviewed timing
+contract. Clock trust and clock rollback are also outside this change. Frozen
+inclusive expiry comparisons remain unchanged.
+
+No live canary was rerun and no present-day branch-rule configuration is
+asserted. Existing canary evidence is historical, scoped evidence. The Windows
+test suite does not certify the fixed Linux controller runtime or a production
+OIDC verifier. Independent evidence here means a reproducible regression using
+a real lock and synthetic inputs; no separate human review is claimed.
+
+## Sources
+
+- [Signer frozen contract](repo-sentinel-signer-v1.md).
+- [Commit-intrinsic authority](repo-sentinel-commit-authority-v1.md).
+- [External policy root](repo-sentinel-external-policy-root-v1.md).
+- Canary repository: `stacknil/repo-sentinel-authority-canary`, inspected
+  commit `d34538c`. Source-binding evidence: `evidence/canary-results.md`.
+  Test-merge rejection: `evidence/test-merge-authority-gate-1.md`.
+- [GitHub OIDC reference](https://docs.github.com/en/actions/reference/security/oidc)
+  documents token time, execution, repository, and workflow claims; it does not
+  prove that this mock core cryptographically verifies them.
+- GitHub Docs, "About protected branches", section "Require status checks
+  before merging", describes expected-App status sources. Canary conclusions
+  above come from the recorded experiments, not a new settings inspection.

signer/repo_sentinel_signer/service.py

@@ -281,8 +281,8 @@ def issue_evaluation(
     ) -> TicketResponse:
         if type(request) is not TicketRequest:
             raise SignerRefused("invalid_ticket_request")
-        now = self._now()
         with self._store.transaction():
+            now = self._now()
             if type(verified_oidc) is not VerifiedOidcClaims:
                 raise SignerRefused("unverified_oidc_claims")
             record = self._registry.resolve_active(
@@ -489,8 +489,8 @@ def finalize_evaluation(
         evaluation_id: str,
         controller_result: object,
     ) -> FinalizationResult:
-        now = self._now()
         with self._store.transaction():
+            now = self._now()
             evaluation = self._store.get(evaluation_id)
             record = self._registry.get(evaluation.registry_key)
             if self._registry.is_revoked(record.key):