Skip to content

[security] fix(release): isolate notarization temp files#1228

Merged
steipete merged 1 commit into
steipete:mainfrom
Hinotoi-agent:fix/notary-key-private-temp
May 30, 2026
Merged

[security] fix(release): isolate notarization temp files#1228
steipete merged 1 commit into
steipete:mainfrom
Hinotoi-agent:fix/notary-key-private-temp

Conversation

@Hinotoi-agent
Copy link
Copy Markdown
Contributor

@Hinotoi-agent Hinotoi-agent commented May 30, 2026

Summary

This PR hardens the release notarization temporary-file boundary in Scripts/sign-and-notarize.sh.

  • Replaces the predictable /tmp/codexbar-api-key.p8 App Store Connect key path with a per-run mktemp -d workspace.
  • Stores the notarization upload ZIP in the same private workspace instead of predictable /tmp/${APP_NAME}Notarize.zip.
  • Creates the private key file under umask 077, explicitly keeps it 0600, and removes the whole workspace on exit.
  • Validates the behavior with a stubbed local notarization run that pre-creates the historical /tmp paths as symlinks and confirms they are not touched.

Security issues covered

Issue Impact Severity
Predictable App Store Connect notarization key path in /tmp A same-host process on a release machine can race or pre-place /tmp/codexbar-api-key.p8 to read or capture the notarization private key used during release signing/notarization. High
Predictable notarization ZIP path in /tmp A same-host process can interfere with or capture the transient notarization archive path used before notarytool submit. Medium

Before this PR

  • The release script wrote APP_STORE_CONNECT_API_KEY_P8 to /tmp/codexbar-api-key.p8.
  • The key file was created at a fixed public path before cleanup ran.
  • The notarization ZIP was also created at a predictable /tmp/${APP_NAME}Notarize.zip path.
  • A pre-existing symlink or same-host watcher could target those names during a privileged release workflow.

After this PR

  • Each release run creates a fresh codexbar-notarize.XXXXXX directory with 0700 permissions.
  • The notary API key is written only inside that private directory and is kept 0600.
  • The notarization ZIP is written inside the same private directory.
  • The EXIT trap removes the private workspace instead of deleting fixed public paths.

Why this matters

Release notarization runs with App Store Connect credentials that should not be exposed to other local users or untrusted same-host processes. Predictable names in a shared temporary directory are a common source of symlink and race issues; on a CI or release host, that can turn a transient secret into a durable credential leak.

The exposed credential is not the Developer ID signing certificate itself, so this PR does not claim a complete signing-key compromise. It does protect the notarization API key and the release archive staging path from predictable shared-directory access.

Attack flow

same-host process on release machine
    -> pre-creates or watches /tmp/codexbar-api-key.p8
        -> release script writes APP_STORE_CONNECT_API_KEY_P8 to that fixed path
            -> process captures the notary API private key before cleanup

Affected code

Issue Files
Predictable notarization key and ZIP temporary paths Scripts/sign-and-notarize.sh

Root cause

Predictable notarization key path:

  • The script wrote a sensitive API key to a fixed path in a shared temporary directory.
  • Cleanup happened only after the path had already been created and used.

Predictable notarization ZIP path:

  • The script staged the notarization archive at a fixed shared-directory path.
  • The path was not scoped to a per-run private workspace.

CVSS assessment

Issue CVSS v3.1 Vector
Predictable App Store Connect notarization key path 7.3 High CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:C/C:H/I:L/A:N

Rationale:

  • The attacker must already have local execution on the release host or CI runner, but the vulnerable path crosses into release credentials that are outside the attacker's normal authority.
  • Confidentiality is high because the transient private key value can be captured.
  • Integrity is limited because the leaked notary credential supports release/notarization workflows but is not, by itself, the Developer ID signing certificate.

Safe reproduction steps

  1. On the vulnerable version, pre-create a symlink at the historical key path:
    ln -sf /tmp/codexbar-captured-key /tmp/codexbar-api-key.p8
  2. Run the release script with stubbed swift, codesign, ditto, xcrun, spctl, stapler, and xattr commands so no real Apple service is contacted.
  3. Observe that the vulnerable script writes the fake APP_STORE_CONNECT_API_KEY_P8 content through the predictable /tmp/codexbar-api-key.p8 path.
  4. On this patched branch, run the same harness with the same symlink already present.
  5. Observe that notarytool receives a key path under a fresh codexbar-notarize.XXXXXX directory, the key is 0600, the directory is 0700, and the historical /tmp symlink remains untouched.

Expected vulnerable behavior

  • The vulnerable script creates or follows /tmp/codexbar-api-key.p8 while handling APP_STORE_CONNECT_API_KEY_P8.
  • A same-host process can target that fixed path before or during the release run.
  • Cleanup does not prevent the key from being exposed during the release window.

Changes in this PR

  • Creates NOTARIZATION_TEMP_DIR with mktemp -d under ${TMPDIR:-/tmp}.
  • Applies chmod 700 to the temporary workspace.
  • Writes API_KEY_PATH inside that workspace under umask 077 and explicitly keeps it chmod 600.
  • Moves NOTARIZATION_ZIP into the same private workspace.
  • Updates xcrun notarytool submit to use the private key and ZIP paths.
  • Replaces fixed-path cleanup with removal of the per-run workspace.

Files changed

Category Files What changed
Release hardening Scripts/sign-and-notarize.sh Uses a private per-run temporary directory for notarization key and ZIP staging.

Maintainer impact

  • The patch is limited to the notarization release script.
  • Signing, packaging, stapling, dSYM packaging, and final release artifact names are unchanged.
  • Operators still provide the same APP_STORE_CONNECT_* environment variables.
  • The temporary notary key and upload ZIP are now isolated from shared /tmp names.

Fix rationale

A per-run private temporary directory is the narrowest durable boundary for this script: it removes predictable names, avoids symlink reuse at shared paths, keeps permissions explicit, and keeps the rest of the release workflow unchanged.

Using mktemp -d also lets the script clean up a single generated workspace rather than relying on fixed file names that could be attacker-controlled.

Type of change

  • Security fix
  • Tests
  • Documentation update
  • Refactor with no behavior change

Test plan

  • bash -n Scripts/sign-and-notarize.sh
  • Stubbed full-script release/notarization run with fake external tools and pre-existing historical /tmp symlinks.
  • Verified the notary key path is under codexbar-notarize.XXXXXX, key mode is 0600, directory mode is 0700, and /tmp/codexbar-api-key.p8 is not created or followed.
  • Verified the script no longer contains references to the historical fixed /tmp key or notarization ZIP paths.
  • git diff --check

Executed with:

  • bash -n Scripts/sign-and-notarize.sh
  • Stubbed local harness for Scripts/sign-and-notarize.sh with ARCHES=arm64, fake Apple/tooling commands, and pre-created historical /tmp symlinks.
  • git grep -n -E '/tmp/(codexbar-api-key\.p8|\$\{APP_NAME\}Notarize\.zip|CodexBarNotarize\.zip)' -- Scripts/sign-and-notarize.sh
  • git diff --check

A real Apple notarization submission was not run because that would require live project release credentials and external service access.

Redacted harness proof

The stubbed notarization harness uses fake APP_STORE_CONNECT_* values, fake Apple/tooling commands, and pre-created historical /tmp symlinks. No live Apple notarization service or real credentials are used.

### stubbed notarization harness
$ ARCHES=arm64 APP_STORE_CONNECT_*=(fake values) DITTO_BIN=$FAKE_DITTO TMPDIR=$PRIVATE_TMP bash Scripts/sign-and-notarize.sh
stub swift build --arch arm64
stub package_app release
Signing with Developer ID Application: Peter Steinberger (Y5PE65HELJ)
stub codesign CodexBar.app/Contents/Helpers/CodexBarCLI
stub codesign CodexBar.app/Contents/Helpers/CodexBarClaudeWatchdog
stub codesign CodexBar.app
notarization_zip_path=$TMPDIR/codexbar-notarize.DWVZdi/CodexBarNotarize.zip
stub ditto wrote $TMPDIR/codexbar-notarize.DWVZdi/CodexBarNotarize.zip
Submitting for notarization
notarytool_key_path=$TMPDIR/codexbar-notarize.DWVZdi/codexbar-api-key.p8
notarytool_key_mode=600
notarytool_temp_dir_mode=700
notarytool_key_content=<redacted>_PRIVATE_KEY_LINE_1
<redacted>_PRIVATE_KEY_LINE_2
stub notarytool submit ok
Stapling ticket
stub xcrun stapler staple CodexBar.app
stub xattr -cr CodexBar.app
stub ditto wrote CodexBar-1.2.3-arm64.zip
stub spctl -a -t exec -vv CodexBar.app
stub stapler validate CodexBar.app
Packaging dSYM
stub ditto wrote CodexBar-1.2.3-arm64.dSYM.zip
Done: CodexBar-1.2.3-arm64.zip

### historical fixed-path symlink checks
historical_key_symlink_target_content=untouched-key-marker
historical_zip_symlink_target_content=untouched-zip-marker
historical_key_symlink_still_exists=true
historical_zip_symlink_still_exists=true
### cleanup check
private_temp_dirs_after_exit=0

### static validation
bash_n=ok
git_diff_check=ok

Disclosure notes

  • This PR is intentionally bounded to local release-host temporary-file handling.
  • It does not claim remote code execution or Developer ID signing certificate compromise.
  • It protects transient App Store Connect notarization credentials and release ZIP staging from predictable shared temporary paths.
  • No unrelated files were changed.

@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 30, 2026

Codex review: needs maintainer review before merge. Reviewed May 30, 2026, 5:27 PM ET / 21:27 UTC.

Summary
The branch changes Scripts/sign-and-notarize.sh to stage the App Store Connect key and notarization ZIP in a private per-run mktemp -d directory with restrictive permissions and cleanup.

Reproducibility: yes. Source inspection of current main shows the release script writes the notary key and upload ZIP to predictable shared /tmp paths, and the PR body provides after-fix terminal proof for the same path boundary.

Review metrics: 2 noteworthy metrics.

  • Diff Surface: 1 file changed, 15 added, 5 deleted. The patch is small, but the single touched file is the signing/notarization script that handles release credentials.
  • Proof Scope: 1 redacted terminal harness transcript. The proof directly exercises the changed temp-path behavior without using live Apple credentials.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

  • [P1] The diff changes how App Store Connect key material is written, permissioned, passed to notarytool, and removed during release signing.
  • [P1] The redacted harness proves the shell temp-path behavior, but a live Apple notarization submission was intentionally not run; macOS release-host TMPDIR, mktemp, and cleanup behavior should be accepted or validated by a release owner.

Maintainer options:

  1. Validate With Release Owner (recommended)
    Have a maintainer who owns releases accept the harness proof or run a credential-safe release dry run before merging.
  2. Accept Stubbed Harness Coverage
    Maintainers may merge with the existing proof if they judge the shell-only temp-path change sufficiently covered without live notarization.
  3. Hold For Release Workflow Batch
    If the release owner wants notarization-script changes grouped with broader release workflow work, pause this PR instead of closing it as obsolete.

Next step before merge

  • [P2] Security-sensitive release automation should get maintainer release-owner review rather than an automated repair lane.

Security
Cleared: The diff narrows predictable temp-file exposure for notarization credentials and adds no dependencies, downloads, broader permissions, or new secret sinks.

Review details

Best possible solution:

Land the private temp-workspace hardening after release-owner review accepts the redacted harness proof or performs an equivalent credential-safe release-path validation.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection of current main shows the release script writes the notary key and upload ZIP to predictable shared /tmp paths, and the PR body provides after-fix terminal proof for the same path boundary.

Is this the best way to solve the issue?

Yes. A per-run private temp directory with 0700 workspace permissions, a 0600 key file, and one cleanup trap is the narrow maintainable fix; the remaining question is release-owner validation, not implementation direction.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against c56619743ab3.

Label changes

Label justifications:

  • P2: This is bounded release-script security hardening with meaningful credential impact but limited normal-user blast radius.
  • merge-risk: 🚨 security-boundary: The diff changes the boundary around App Store Connect private key material during notarization.
  • merge-risk: 🚨 automation: The diff changes release automation paths for the notarization ZIP and key file, which ordinary CI cannot fully validate.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes redacted terminal output from a stubbed full-script notarization run showing the fixed temp paths, restrictive modes, untouched historical symlinks, and cleanup.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes redacted terminal output from a stubbed full-script notarization run showing the fixed temp paths, restrictive modes, untouched historical symlinks, and cleanup.
Evidence reviewed

What I checked:

  • Repository policy read: AGENTS.md was read in full; its release-script and release-key guidance applies because this PR touches signing/notarization automation. (AGENTS.md:24, c56619743ab3)
  • Current main vulnerable key path: Current main writes APP_STORE_CONNECT_API_KEY_P8 to /tmp/codexbar-api-key.p8, so the central hardening is not already present on main. (Scripts/sign-and-notarize.sh:20, c56619743ab3)
  • Current main vulnerable ZIP path: Current main stages and submits the notarization ZIP at /tmp/${APP_NAME}Notarize.zip, matching the predictable shared temp path described by the PR. (Scripts/sign-and-notarize.sh:55, c56619743ab3)
  • PR implementation: The PR head creates NOTARIZATION_TEMP_DIR with mktemp -d, chmods it 700, writes the key under umask 077, chmods the key 600, and submits the private ZIP/key paths to notarytool. (Scripts/sign-and-notarize.sh:21, ca13d7db9892)
  • PR patch scope: The current PR patch modifies only Scripts/sign-and-notarize.sh with 15 insertions and 5 deletions around the notarization temp paths. (Scripts/sign-and-notarize.sh:17, ca13d7db9892)
  • Real behavior proof: The PR body includes redacted terminal output from a stubbed full-script notarization run showing private temp paths, 0600 key mode, 0700 directory mode, untouched historical symlinks, and cleanup after exit. (ca13d7db9892)

Likely related people:

  • steipete: Current blame and GitHub file history for Scripts/sign-and-notarize.sh point to Peter Steinberger / steipete for the signing/notarization script and recent release automation changes. (role: release-script owner and recent area contributor; confidence: high; commits: f83fb70b7f24, 487a78ceda7d, 4daacac3ca07; files: Scripts/sign-and-notarize.sh, .agents/skills/release-codexbar/SKILL.md)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. labels May 30, 2026
@Hinotoi-agent
Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 30, 2026

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels May 30, 2026
@steipete steipete force-pushed the fix/notary-key-private-temp branch from c97ffd6 to ca13d7d Compare May 30, 2026 21:23
@steipete steipete merged commit e7d9326 into steipete:main May 30, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants