Skip to content

fix(gates): qualify the trusted signing lane on a real signature - #59

Merged
m-szymanska merged 3 commits into
mainfrom
fix/pensieve-gates-keychain-unlock
Aug 16, 2026
Merged

fix(gates): qualify the trusted signing lane on a real signature#59
m-szymanska merged 3 commits into
mainfrom
fix/pensieve-gates-keychain-unlock

Conversation

@m-szymanska

@m-szymanska m-szymanska commented Aug 16, 2026

Copy link
Copy Markdown
Member

Third blocker of the headless release (family of #55 / #58): make gates
test-scriptsscripts/test-isolated-app.sh failed with "could not sign
the main source fixture"
on the release machine over SSH.

Evidence chain

  1. scripts/test-isolated-app.sh qualified its trusted lane on the mere
    presence of a Developer ID identity:
    TRUSTED_SIGNING_IDENTITY="$(security find-identity -v -p codesigning …)".
  2. Non-empty → sign_source_fixture "$TRUSTED_SIGNING_IDENTITY"codesign
    fail.
  3. The identity lives only in the dedicated pensieve-build.keychain-db, which
    was locked for that session. security find-identity still lists it:
    the certificate is public, only the private key is sealed — the exact trap
    AGENTS.md documents after fix(release): unlock the build keychain in the session that runs codesign #55.
  4. The auto-unlock added in fix(release): unlock the build keychain in the session that runs codesign #55 lives in scripts/build-release.sh and starts
    only when a release lane runs — that is, after the gates. So the gates
    reached for a keychain nothing had opened yet.

A documented certless skip lane already existed
(PENSIEVE_TEST_FORCE_CERTLESS=1); CI falls into it naturally because it has no
identity at all. Only a machine that has the identity but cannot use it hit
the hard failure.

Fix

1. scripts/lib/build-keychain.sh (new). The build-keychain preflight moves
out of build-release.sh verbatim, behind the same fence markers.
build-release.sh sources it; behaviour there is unchanged. KEYS_DIR gains
the same default it was previously handed, so a caller under set -u that has
no reason to know about ~/.keys can source the lib.

2. Opportunistic unlock + capability-based qualification.
test-isolated-app.sh sources the lib for the non-fatal
unlock_build_keychain alone — it is a test suite, not the release gate, so
preflight_build_keychain (which may die) is deliberately not called. It then
qualifies the trusted lane on a trial codesign of a throwaway Mach-O
(--timestamp=none, nothing reaches the network) instead of on the listing. A
listed-but-unusable identity is reported with the codesign error and
downgraded to the existing certless skip lane, never a failure. CI, with no
identity, skips exactly as before.

This is not a new prompt surface: the trusted lane already drives codesign
with this identity dozens of times. The probe only moves the first attempt to a
cheap, self-describing place — and in a GUI session, cancelling a panel now
downgrades to the skip lane instead of failing a gate.

3. No new security call can raise a GUI panel. The suite calls only
unlock_build_keychain, which always passes -p and never reaches
SecurityAgent. security show-keychain-info — the one call that does raise a
modal against a locked keychain — stays fenced inside
preflight_build_keychain's GUI-session branch in the lib, uncalled from here.
No new files holding secrets.

4. Helper-list trap (lesson of #54). The new helper is a release runtime
input, so it is sealed into every enumeration at once: the snapshot archive
in build-release.sh, the existence / digest / status lists in
build-provenance.sh, and the dirty-input list in isolated-app.sh, plus a
fixture in the provenance fixture repo. test-landing-page.sh enforces this
structurally.

5. Docs. AGENTS.md's Release section names the helper and the new trusted-
lane qualification.

Gates

  • bash -n and shellcheck -x: clean on all touched files (baseline was clean).
  • ./scripts/test-build-keychain.sh46 passed, 0 failed, intent
    unmodified; the fence is extracted from the lib and the structural assertions
    stay on build-release.sh, with a new hard check that it really sources the
    lib.
  • ./scripts/test-landing-page.sh66 passed, 0 failed, including the
    helper-list assertion. Mutation-checked: dropping build-keychain.sh from the
    isolated-app.sh list turns it into 2 failures.
  • ./scripts/test-isolated-app.sh — runtime proof on the release machine over
    SSH, see the runtime note below.

Runtime proof (release machine, over SSH)

All three runs on the release machine over SSH — the exact session shape that
produced the failure.

Baseline, main @ 114a99a — reproduces the reported blocker:

[isolated-app test PASS] normalized payload provenance survives executable rename and re-signing
[isolated-app test FAIL] could not sign the main source fixture

This branch — the trusted lane is executed, not skipped, and the whole
suite is green headless (48 passed, 0 failed, "all synthetic checks passed"):

[isolated-app test PASS] normalized payload provenance survives executable rename and re-signing
[isolated-app test] build keychain unlocked (or absent) for this session
[isolated-app test PASS] source provenance accepts the current clean trusted runtime input set
[isolated-app test PASS] normalized payload provenance survives Developer ID to ad-hoc re-signing

This is also the first runtime execution of #55's unlock logic through the
extracted helper: it fires and works over SSH.

Mutation check — the same branch with only the unlock_build_keychain call
removed. The trial signature catches it, names the cause, and downgrades to the
certless lane instead of failing:

[isolated-app test SKIP] Developer ID identity [Developer ID Application: … (MW223P3NPX)] is listed
  but cannot sign in this session: … errSecInternalComponent
[isolated-app test SKIP] trusted source staging lane needs Developer ID
[isolated-app test] all certificate-free synthetic checks passed

So the unlock call is load-bearing (without it the trusted lane is unreachable),
and the probe converts what used to be a hard gate failure into a readable skip.
Note the baseline failure reproduces even though an earlier SSH session on the
same host had already unlocked that keychain — the unlock genuinely does not
carry across security sessions, which is why it has to happen in-process.

Operator machine (GUI session): ./scripts/test-isolated-app.sh — 48 passed,
0 failed, trusted lane executed.

…on a real signature

- Extract the build-keychain preflight from build-release.sh into the new
  sourceable helper scripts/lib/build-keychain.sh, verbatim and behind the same
  fence markers. build-release.sh sources it; behaviour is unchanged.
- Source it from scripts/test-isolated-app.sh for the opportunistic, non-fatal
  `unlock_build_keychain` only. `make gates` runs long before any release lane,
  so the suite was reaching for a Developer ID identity out of a build keychain
  nothing had opened yet — headless `make gates` died on "could not sign the
  main source fixture" while the release lane, which unlocks for itself, was
  fine.
- Qualify the trusted lane on a trial `codesign` of a throwaway Mach-O instead
  of on `security find-identity`. The certificate is public and stays listed out
  of a LOCKED keychain, so the listing never was evidence that signing works. A
  listed-but-unusable identity now downgrades to the existing certless skip
  lane, with the codesign error printed, rather than failing the gate. CI, with
  no identity at all, skips exactly as before.
- Seal the new helper into every release-helper enumeration at once (the
  snapshot archive, build-provenance.sh's existence/digest/status lists,
  isolated-app.sh's dirty-input list) and add its fixture to the provenance
  fixture repo. test-landing-page.sh asserts that structurally.
- test-build-keychain.sh extracts the fence from the lib and gains a hard check
  that build-release.sh really sources it; its structural assertions stay on
  build-release.sh, where the lane flag and signing sites live.
- Document the helper and the new trusted-lane qualification in AGENTS.md.
- Gates: bash -n + shellcheck -x clean on all touched files (baseline clean),
  ./scripts/test-build-keychain.sh 46/46, ./scripts/test-landing-page.sh 66/66.

Authored-By: claude <agents@vetcoders.io>

session_id: 01a0057d-7dd8-7421-8363-844e9cf10946

time: 2026-08-16T03:51:08+02:00

runtime: interactive
Copilot AI lite review requested due to automatic review settings August 16, 2026 01:54

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Code review skipped — your organization has reached its monthly code review spending cap.

An organization admin can view or raise the cap at claude.ai/admin-settings/claude-code. The cap resets at the start of the next billing period.

Once the cap resets or is raised, reopen this pull request to trigger a review.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the release/test “trusted signing” gates so the trusted lane is qualified based on actual signing capability (not merely identity presence), addressing headless/SSH failures caused by locked build keychains.

Changes:

  • Extracts build-keychain unlock/preflight logic into a shared helper (scripts/lib/build-keychain.sh) and sources it from scripts/build-release.sh and scripts/test-isolated-app.sh.
  • Makes scripts/test-isolated-app.sh perform an opportunistic unlock and qualify the trusted lane via a trial codesign probe, downgrading to the certless lane when signing isn’t possible.
  • Seals the new helper into all required “runtime input” enumerations and updates script tests/docs accordingly.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
scripts/lib/build-keychain.sh New shared build-keychain unlock/preflight helper used by both release and gates.
scripts/build-release.sh Sources the shared helper and includes it in release snapshot inputs.
scripts/test-isolated-app.sh Opportunistically unlocks build keychain and qualifies trusted lane via a trial signature.
scripts/test-build-keychain.sh Updates extraction/testing to read the fenced block from the new helper and asserts it’s sourced by build-release.
scripts/test-landing-page.sh Adds the new helper to the structural “all helpers enumerated everywhere” assertion set.
scripts/lib/build-provenance.sh Adds the new helper to provenance runtime input status/cleanliness/digest enumerations.
scripts/lib/isolated-app.sh Adds the new helper to isolated-app runtime input status enumeration.
AGENTS.md Documents the new helper and the capability-based trusted-lane qualification behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread scripts/lib/build-keychain.sh Outdated
Comment on lines +137 to +143
die "Build-keychain password file is readable beyond its owner: $BUILD_KEYCHAIN_PASSWORD_FILE
It holds the keychain password in cleartext and \$HOME is
world-executable, so every local account can read it. Refusing to use it
until it is the 0600 secret it is documented to be:
chmod 600 \"$BUILD_KEYCHAIN_PASSWORD_FILE\"
Treat the stored password as disclosed: change it on the keychain and
re-store it."

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in de00ef9 — the status-3 diagnostic now names every cause the privacy check actually catches (foreign owner, unreadable stat metadata, any group/other permission bits) and states both accepted owner-only modes (0600, or 0400 for a read-only copy). The remedy adds ls -l and a chown line for the wrong-owner case; detection logic untouched.

…d-file privacy diagnostic (PR #59 Copilot)

- The status=3 branch fires on three distinct conditions — a file owned by
  another user, owner/mode that `stat -L` could not read at all, and any
  group or other permission bit — but the message named only the last one
  ("readable beyond its owner") and offered `chmod 600` as the whole remedy.
- Rewrite the diagnostic to list all three causes and to state both accepted
  modes (0600, or 0400 for a deliberately read-only file), matching what
  build_keychain_password_file_is_private actually enforces.
- Extend the remedy with `ls -l` and `chown` so a wrong-owner or unreadable
  file has a fix, keeping the `chmod 600 "<file>"` line the test pins, and
  scope the "treat the password as disclosed" warning to the cases where the
  file really was reachable by another account.
- Update the status legend above unlock_build_keychain to match.
- Detection logic untouched; scripts/test-build-keychain.sh stays 46/46.

Authored-By: claude <agents@vetcoders.io>

session_id: 01a0057d-7dd8-7421-8363-844e9cf10946

time: 2026-08-16T04:14:07+02:00

runtime: interactive
Copilot AI review requested due to automatic review settings August 16, 2026 02:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: de00ef9af2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/test-isolated-app.sh Outdated
# So: unlock first (opportunistic and non-fatal — this is a test suite, not the
# release gate), then qualify on a real trial signature.
BUILD_KEYCHAIN_UNLOCK_STATUS=0
unlock_build_keychain || BUILD_KEYCHAIN_UNLOCK_STATUS=$?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep gate tests away from the real build keychain

When make test-scripts runs on a Mac with the default dedicated keychain, this call reads the operator's real password file and invokes security unlock-keychain, mutating the real security session from a unit/integration gate. This also happens when PENSIEVE_TEST_FORCE_CERTLESS=1, before the script checks that flag, so even the explicitly certificate-free lane touches production keychain state. Inject or shim this capability, or keep the real unlock in an opt-in runtime smoke rather than the gate.

AGENTS.md reference: AGENTS.md:L163-L171

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in e3c8029 (ordering) — the whole identity resolution now sits behind the PENSIEVE_TEST_FORCE_CERTLESS check, so the certless lane performs zero security calls of any kind (the unconditional find-identity moved too, since it ran before the flag check as well). Enforced at runtime by counter assertions right after the decision: certless requires 0 unlocks / 0 lookups, the trusted lane exactly 1/1; a mutant with the old ordering fails with 'the certless lane unlocked the real build keychain'.

Declined the shim/opt-in-smoke half deliberately: the trusted lane's contract is real signing with the real identity (it invokes codesign with it throughout), so shimming the unlock would make the lane test a fiction. The opportunistic unlock is the minimal enabling state change — the same one an operator performed manually before this PR — and it now runs only on the path that can actually enter that lane.

…y from the real keychain (PR #59 Codex)

- Check PENSIEVE_TEST_FORCE_CERTLESS before the opportunistic build-keychain
  unlock instead of after it. An explicitly certificate-free run no longer
  mutates the state of the operator's real keychain for a lane that then
  discards the identity anyway.
- Move the `security find-identity` listing into the same else-branch: the
  certless lane now reaches no `security` invocation at all, not just no
  codesign.
- Leave the unlock unconditional on the trusted branch. unlock_build_keychain
  already returns 0 when no dedicated build keychain exists, so a second
  existence check here would only duplicate its contract.
- Assert the new order at runtime: counters record what actually ran, so the
  certless lane fails on a non-zero unlock or identity-lookup count and the
  trusted lane fails on anything other than exactly one of each. Reverting the
  hoist fails the certless assertion instead of silently touching the keychain.

Authored-By: claude <agents@vetcoders.io>

session_id: 01a0057d-7dd8-7421-8363-844e9cf10946

time: 2026-08-16T04:50:02+02:00

runtime: interactive
Copilot AI review requested due to automatic review settings August 16, 2026 02:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

@m-szymanska
m-szymanska merged commit 1455c8c into main Aug 16, 2026
5 checks passed
@m-szymanska
m-szymanska deleted the fix/pensieve-gates-keychain-unlock branch August 16, 2026 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants