Skip to content

fix: make macOS Keychain repair action-based #70

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rianjs wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: make macOS Keychain repair action-based #70

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,9 @@ jobs:
- uses: golangci/golangci-lint-action@v7

@piekstra-dev piekstra-dev Jul 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

File-level note: .github/workflows/ci.yml

ci.md §1 requires a concurrency group keyed on the ref with cancel-in-progress: true (MUST). The workflow has no concurrency block, so rapid pushes or re-pushes on the same ref queue redundant runs rather than cancelling the superseded one. This is pre-existing, but this PR modifies ci.yml and is the natural place to add it.

Suggested addition immediately after the on: block:

concurrency:
  group: ci-${{ github.ref }}
  cancel-in-progress: true

Reply inline to this comment.

with:
version: v2.12.2

test-scripts:
runs-on: ubuntu-latest

@piekstra-dev piekstra-dev Jul 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The test-scripts job runs only on ubuntu-latest, but repair-macos-keychain-credentials.sh is a macOS Keychain script. If --self-test detects a non-macOS host and early-exits (the common pattern for macOS-specific repair scripts), this job validates only that the script is bash-syntactically valid and that --self-test doesn't crash on Linux — not that any repair logic executes correctly. The bash -n step is genuinely cross-platform useful, but for the self-test to provide meaningful automation coverage it needs a macOS runner.

Fix: change the runner to macos-latest, or add a matrix so the job runs on both ubuntu-latest (for the syntax check) and macos-latest (for the self-test logic). If --self-test is intentionally designed to exercise full logic cross-platform without hitting macOS APIs, add a comment to the workflow step explaining that so the choice is not silently reversed by a future editor.

Reply inline to this comment.

steps:
- uses: actions/checkout@v4
- run: make test-scripts
8 changes: 6 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
.PHONY: check tidy lint test build
.PHONY: check tidy lint test test-scripts build

# CI gate: everything that must pass before merge (repo-layout.md §2.1 —
# check mirrors CI, including the build step).
check: tidy lint test build
check: tidy lint test test-scripts build

# Tidy and verify the module is clean. Catches both modified tracked and
# newly-generated untracked go.mod/go.sum (the latter once deps are added).
Expand All @@ -17,5 +17,9 @@ lint:
test:
go test -race ./...

test-scripts:
bash -n scripts/repair-macos-keychain-credentials.sh
scripts/repair-macos-keychain-credentials.sh --self-test

build:
go build ./...
5 changes: 4 additions & 1 deletion docs/development.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ profile** in [`repo-layout.md`](repo-layout.md) §2.1.
## Build / test

```sh
make check # tidy + lint + test + build — mirrors CI
make check # tidy + lint + test + test-scripts + build — mirrors CI
make test-scripts # run support-script syntax and fixture tests
```

Requires Go per the `go` directive in `go.mod` (the single version source,
Expand All @@ -35,6 +36,8 @@ Tests MUST be hermetic: use the in-memory credstore backend
OS keychain or home directories; that class of leak is exactly what
`statedirtest` exists to prevent.

Support scripts are documented in [`../scripts/README.md`](../scripts/README.md).

## Releases

No auto-release, no `version.txt` (library profile, `repo-layout.md` §2.1).
Expand Down
68 changes: 68 additions & 0 deletions scripts/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
# Scripts

@piekstra-dev piekstra-dev Jul 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

File-level note: scripts/README.md

The README documents --heal, --cleanup, --rebuild, --apply, and --tool but omits --keychain PATH, which the script supports and lists in its usage string. A user who needs to target a non-default keychain path has no indication this flag exists. Add a short example such as:

scripts/repair-macos-keychain-credentials.sh --keychain ~/Library/Keychains/login.keychain-db

Reply inline to this comment.


Shared support scripts for Open CLI Collective repositories.

## `repair-macos-keychain-credentials.sh`

Repairs macOS Keychain generic-password ACLs for Collective CLI credentials
that still trust ad-hoc or per-build `cdhash` identities instead of the current
stable-signed CLI binaries.

Default mode is inspect-only:

```bash
scripts/repair-macos-keychain-credentials.sh
```

Preview an additive heal:

```bash
scripts/repair-macos-keychain-credentials.sh --heal
```

Apply an additive heal:

```bash
scripts/repair-macos-keychain-credentials.sh --heal --apply
```

Clean up already-healed `stable+stale-cdhash` items by rebuilding them into
canonical metadata and stable app ACLs:

```bash
scripts/repair-macos-keychain-credentials.sh --cleanup
scripts/repair-macos-keychain-credentials.sh --cleanup --apply
```

Use the heavy rebuild path when an item should be recreated canonically from its
current secret value:

```bash
scripts/repair-macos-keychain-credentials.sh --rebuild --tool nrq
scripts/repair-macos-keychain-credentials.sh --rebuild --tool nrq --apply
```

Limit discovery to one or more tools:

@piekstra-dev piekstra-dev Jul 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The --tool section shows example usage with cr and nrq but omits the list of valid values. The script's usage string documents them as: slck, nrq, gro, jtk, cfl, atlassian, cr, sfdc. Without this list in the README a reader must run --help or open the script to discover what tools are supported. Add a brief note after the example, e.g.: "Supported values: slck, nrq, gro, jtk, cfl, atlassian, cr, sfdc."

Reply inline to this comment.


```bash
scripts/repair-macos-keychain-credentials.sh --tool cr --tool nrq
```

Run this as the normal macOS user who owns the login Keychain, not with `sudo`.
Real mutation requires `--apply` plus exactly one action: `--heal`, `--cleanup`,
or `--rebuild`. `--apply` alone exits without scanning or mutating.

`--heal --apply` does not read, print, delete, or recreate secret values. It is
intentionally additive: it appends missing stable-signed trusted application
grants to explicit decrypt ACL app lists and preserves existing trusted
applications. It skips `NULL` or non-explicit app-list ACLs instead of narrowing
their meaning.

`--cleanup --apply` and `--rebuild --apply` read the existing secret value,
delete the old Keychain item, and recreate it with canonical label,
description, and stable-signed app ACLs. They never print secrets or pass them
as process arguments.

`stable+stale-cdhash` means the current stable-signed app is already trusted.
macOS may still report older cdhash grants or partition metadata for that item;
the script treats that state as repaired.
Loading
Loading