Skip to content

fix: remove darwin probe seed traversal - #94

Merged
rianjs merged 1 commit into
mainfrom
bug/83-darwin-keychain-seed
Jun 3, 2026
Merged

fix: remove darwin probe seed traversal#94
rianjs merged 1 commit into
mainfrom
bug/83-darwin-keychain-seed

Conversation

@rianjs

@rianjs rianjs commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • switch the darwin keychain probe seed to seed_config.base: native_user_config
  • remove the ../Library/Application Support/... traversal from seed_config.path
  • keep the released darwin arm64 assertion for backend_source: auto selecting Keychain

Shared automation

Verification

  • rg --hidden -n '\\.\\.|native_user_config|seed_config|backend_source' packaging/identity.yml\n- uv run --with PyYAML==6.0.2 --with defusedxml==0.7.1 python /Users/rianjs/dev/.github/actions/identity-check/identity.py validate --working-dir . --repo-root .\n- actionlint .github/workflows/ci.yml .github/workflows/release.yml .github/workflows/auto-release.yml\n- go test ./...\n- golangci-lint run --path-prefix=.\n\nCloses Remove path traversal from darwin keychain probe seed config #83

@rianjs

rianjs commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Major

  • The current verification still does not exercise the actual darwin arm64 probe. packaging/identity.yml now seeds native_user_config, but the listed checks (rg, identity.py validate, actionlint, go test, golangci-lint) are all structural or Linux-hosted. They would still pass if the seed landed in the wrong directory or if backend_source:auto stopped resolving to Keychain on macOS arm64. The shared distribution standard says the functional check is a release-time Apple Silicon run, and identity-check does not verify the keychain probe itself because that data is generated directly from the manifest. See distribution.md and distribution.md. Add one focused macOS arm64 smoke test or equivalent release-workflow coverage for this stanza.

@monit-reviewer monit-reviewer 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.

Automated PR Review

Reviewed commit: d833cd4

Approved with 1 non-blocking suggestion below. Address at your discretion.

Summary

Reviewer Findings
harness-engineering:harness-architecture-reviewer 1
harness-engineering:harness-architecture-reviewer (1 findings)

💡 Suggestion - packaging/identity.yml:22

The functional correctness of base: native_user_config resolving to the correct path and backend_source: auto selecting Keychain on darwin arm64 cannot be verified by the listed checks (rg, identity.py validate, actionlint, go test, golangci-lint), which are all structural or Linux-hosted. Per the PR author's own comment, the shared distribution standard defers this to a release-time Apple Silicon run — meaning a silent regression is possible if native_user_config resolves differently than the old traversal in the hermetic HOME environment. Consider documenting the expected hermetic HOME resolution behavior of native_user_config in a comment or accompanying doc.

1 PR discussion thread considered.


Completed in 22s | $0.16 | sonnet | daemon 0.2.121 | Glorfindel
Field Value
Model sonnet
Reviewers hybrid-synthesis, harness-engineering:harness-architecture-reviewer, harness-engineering:harness-enforcement-reviewer, harness-engineering:harness-knowledge-reviewer
Engine claude · sonnet
Reviewed by pr-review-daemon · monit-pr-reviewer
Duration 22s wall · 25s compute (Reviewers: 11s · Synthesis: 8s)
Cost $0.16
Tokens 58.8k in / 958 out
Turns 4

Per-workstream usage

Workstream Model In Out Cache read Cache create Cost
hybrid-synthesis sonnet 30.2k 297 18.6k 11.5k (1h) $0.06
harness-engineering:harness-architecture-reviewer sonnet 9.5k 420 2.1k 7.4k (1h) $0.04
harness-engineering:harness-enforcement-reviewer sonnet 9.5k 100 2.1k 7.4k (1h) $0.03
harness-engineering:harness-knowledge-reviewer sonnet 9.5k 141 2.1k 7.4k (1h) $0.03

Re-reviews only run when @monit-reviewer is re-requested as a reviewer — push as many commits as you need, then re-request when ready. PRs targeting branches other than main, master are skipped, even when @monit-reviewer is re-requested.

Comment thread packaging/identity.yml
# with hermetic HOME that is "$HOME/Library/Application Support". The shared
# darwin gate's native_user_config seed base writes under that root without
# path traversal.
keychain_probe:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Low (harness-engineering:harness-architecture-reviewer): The functional correctness of base: native_user_config resolving to the correct path and backend_source: auto selecting Keychain on darwin arm64 cannot be verified by the listed checks (rg, identity.py validate, actionlint, go test, golangci-lint), which are all structural or Linux-hosted. Per the PR author's own comment, the shared distribution standard defers this to a release-time Apple Silicon run — meaning a silent regression is possible if native_user_config resolves differently than the old traversal in the hermetic HOME environment. Consider documenting the expected hermetic HOME resolution behavior of native_user_config in a comment or accompanying doc.

Reply to this thread when addressed.

@rianjs
rianjs merged commit c2baef5 into main Jun 3, 2026
8 checks passed
@rianjs
rianjs deleted the bug/83-darwin-keychain-seed branch June 3, 2026 01:37
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