diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2aba5d7..efe6d03 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -88,6 +88,12 @@ jobs: - name: Test if: runner.os != 'Linux' run: go test -race -shuffle=on ./... + - name: Test native macOS Keychain integration + if: runner.os == 'macOS' + env: + CGO_ENABLED: "1" + CONED_KEYCHAIN_TEST: "1" + run: go test -race -shuffle=on ./internal/securestore -run '^TestDarwinKeyringIntegration' -count=1 - run: go vet ./... - run: git diff --check lint: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e974a9a..95b2485 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -3,13 +3,14 @@ on: push: tags: ["v*"] permissions: - contents: write - id-token: write - attestations: write + contents: read +concurrency: + group: release-${{ github.ref }} + cancel-in-progress: false jobs: - goreleaser: + preflight: runs-on: ubuntu-latest - timeout-minutes: 20 + timeout-minutes: 30 steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: @@ -19,11 +20,36 @@ jobs: with: go-version-file: go.mod cache: true + - name: Validate native release layout + shell: bash + run: | + set -euo pipefail + if grep -Fq 'goos: [linux, darwin]' .goreleaser.yml && grep -Fq 'CGO_ENABLED=0' .goreleaser.yml; then + echo 'Darwin is still configured as a CGO-disabled Ubuntu cross-build' >&2 + exit 1 + fi + test -f .goreleaser.darwin-amd64.yml + test -f .goreleaser.darwin-arm64.yml + grep -Fq 'goos: [linux]' .goreleaser.yml + grep -Fq 'goarch: [amd64, arm64]' .goreleaser.yml + grep -Fq 'env: [CGO_ENABLED=0]' .goreleaser.yml + grep -Fq 'goos: [darwin]' .goreleaser.darwin-amd64.yml + grep -Fq 'goarch: [amd64]' .goreleaser.darwin-amd64.yml + grep -Fq 'env: [CGO_ENABLED=1]' .goreleaser.darwin-amd64.yml + grep -Fq 'goos: [darwin]' .goreleaser.darwin-arm64.yml + grep -Fq 'goarch: [arm64]' .goreleaser.darwin-arm64.yml + grep -Fq 'env: [CGO_ENABLED=1]' .goreleaser.darwin-arm64.yml + grep -Fxq ' runs-on: macos-15-intel' .github/workflows/release.yml + grep -Fxq ' runs-on: macos-15' .github/workflows/release.yml - name: Validate release tag shell: bash run: | - [[ "$GITHUB_REF_NAME" =~ ^v[0-9]+\.[0-9]+\.[0-9]+([+-][0-9A-Za-z.-]+)?$ ]] - test "$(git cat-file -t "$GITHUB_REF_NAME")" = tag + set -euo pipefail + tag="$GITHUB_REF_NAME" + [[ "$GITHUB_REF" == "refs/tags/$tag" ]] + [[ "$tag" =~ ^v[0-9]+\.[0-9]+\.[0-9]+([+-][0-9A-Za-z.-]+)?$ ]] + test "$(git cat-file -t "$tag")" = tag + test "$(git rev-parse "$tag^{commit}")" = "$GITHUB_SHA" - run: go mod verify - name: Check formatting shell: bash @@ -32,18 +58,299 @@ jobs: test -z "$unformatted" - run: go test -race -shuffle=on ./... - run: go vet ./... - - run: go install golang.org/x/vuln/cmd/govulncheck@v1.6.0 + - uses: goreleaser/goreleaser-action@f06c13b6b1a9625abc9e6e439d9c05a8f2190e94 # v7.2.3 + with: + distribution: goreleaser + version: v2.17.0 + args: check --config .goreleaser.yml + - uses: goreleaser/goreleaser-action@f06c13b6b1a9625abc9e6e439d9c05a8f2190e94 # v7.2.3 + with: + distribution: goreleaser + version: v2.17.0 + args: check --config .goreleaser.darwin-amd64.yml + - uses: goreleaser/goreleaser-action@f06c13b6b1a9625abc9e6e439d9c05a8f2190e94 # v7.2.3 + with: + distribution: goreleaser + version: v2.17.0 + args: check --config .goreleaser.darwin-arm64.yml + - name: Install govulncheck + run: go install golang.org/x/vuln/cmd/govulncheck@v1.6.0 - run: govulncheck ./... + + build-linux: + needs: preflight + runs-on: ubuntu-latest + timeout-minutes: 20 + env: + CGO_ENABLED: "0" + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + fetch-depth: 0 + persist-credentials: false + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0 + with: + go-version-file: go.mod + cache: true + - name: Verify Linux build host + shell: bash + run: | + set -euo pipefail + test "$(uname -s)" = Linux + test "$(uname -m)" = x86_64 + test "$CGO_ENABLED" = 0 + test "$(go env GOOS)" = linux - uses: goreleaser/goreleaser-action@f06c13b6b1a9625abc9e6e439d9c05a8f2190e94 # v7.2.3 with: distribution: goreleaser version: v2.17.0 - args: release --clean + args: release --clean --skip=publish --config .goreleaser.yml + - name: Verify Linux archives + shell: bash + run: | + set -euo pipefail + shopt -s nullglob + archives=(dist/coned-cli_*_linux_*.tar.gz) + test "${#archives[@]}" -eq 2 + test "$(find dist -maxdepth 1 -type f -name '*_linux_amd64.tar.gz' | wc -l | tr -d ' ')" = 1 + test "$(find dist -maxdepth 1 -type f -name '*_linux_arm64.tar.gz' | wc -l | tr -d ' ')" = 1 + test ! -e dist/checksums.txt + - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: coned-release-linux-amd64 + path: dist/coned-cli_*_linux_amd64.tar.gz + if-no-files-found: error + retention-days: 1 + - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: coned-release-linux-arm64 + path: dist/coned-cli_*_linux_arm64.tar.gz + if-no-files-found: error + retention-days: 1 + + build-darwin-amd64: + needs: preflight + runs-on: macos-15-intel + timeout-minutes: 20 + env: + CGO_ENABLED: "1" + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + fetch-depth: 0 + persist-credentials: false + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0 + with: + go-version-file: go.mod + cache: true + - name: Verify native Darwin amd64 build host + shell: bash + run: | + set -euo pipefail + test "$(uname -s)" = Darwin + test "$(uname -m)" = x86_64 + test "$CGO_ENABLED" = 1 + test "$(go env GOOS)" = darwin + test "$(go env GOARCH)" = amd64 + test "$(go env CGO_ENABLED)" = 1 + - uses: goreleaser/goreleaser-action@f06c13b6b1a9625abc9e6e439d9c05a8f2190e94 # v7.2.3 + with: + distribution: goreleaser + version: v2.17.0 + args: release --clean --skip=publish --config .goreleaser.darwin-amd64.yml + - name: Verify Darwin amd64 archive + shell: bash + run: | + set -euo pipefail + shopt -s nullglob + archives=(dist/coned-cli_*_darwin_amd64.tar.gz) + test "${#archives[@]}" -eq 1 + test ! -e dist/checksums.txt + - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: coned-release-darwin-amd64 + path: dist/coned-cli_*_darwin_amd64.tar.gz + if-no-files-found: error + retention-days: 1 + + build-darwin-arm64: + needs: preflight + runs-on: macos-15 + timeout-minutes: 20 + env: + CGO_ENABLED: "1" + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + fetch-depth: 0 + persist-credentials: false + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0 + with: + go-version-file: go.mod + cache: true + - name: Verify native Darwin arm64 build host + shell: bash + run: | + set -euo pipefail + test "$(uname -s)" = Darwin + test "$(uname -m)" = arm64 + test "$CGO_ENABLED" = 1 + test "$(go env GOOS)" = darwin + test "$(go env GOARCH)" = arm64 + test "$(go env CGO_ENABLED)" = 1 + - uses: goreleaser/goreleaser-action@f06c13b6b1a9625abc9e6e439d9c05a8f2190e94 # v7.2.3 + with: + distribution: goreleaser + version: v2.17.0 + args: release --clean --skip=publish --config .goreleaser.darwin-arm64.yml + - name: Verify Darwin arm64 archive + shell: bash + run: | + set -euo pipefail + shopt -s nullglob + archives=(dist/coned-cli_*_darwin_arm64.tar.gz) + test "${#archives[@]}" -eq 1 + test ! -e dist/checksums.txt + - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: coned-release-darwin-arm64 + path: dist/coned-cli_*_darwin_arm64.tar.gz + if-no-files-found: error + retention-days: 1 + + publish: + needs: [preflight, build-linux, build-darwin-amd64, build-darwin-arm64] + if: ${{ always() }} + runs-on: ubuntu-latest + timeout-minutes: 20 + permissions: + contents: write + id-token: write + attestations: write + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + fetch-depth: 0 + persist-credentials: false + - name: Validate prerequisites and release tag + if: ${{ always() }} + env: + PREFLIGHT_RESULT: ${{ needs.preflight.result }} + LINUX_RESULT: ${{ needs.build-linux.result }} + DARWIN_AMD64_RESULT: ${{ needs.build-darwin-amd64.result }} + DARWIN_ARM64_RESULT: ${{ needs.build-darwin-arm64.result }} + RELEASE_TAG: ${{ github.ref_name }} + shell: bash + run: | + set -euo pipefail + test "$PREFLIGHT_RESULT" = success + test "$LINUX_RESULT" = success + test "$DARWIN_AMD64_RESULT" = success + test "$DARWIN_ARM64_RESULT" = success + [[ "$GITHUB_REF" == "refs/tags/$RELEASE_TAG" ]] + [[ "$RELEASE_TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+([+-][0-9A-Za-z.-]+)?$ ]] + test "$(git cat-file -t "$RELEASE_TAG")" = tag + test "$(git rev-parse "$RELEASE_TAG^{commit}")" = "$GITHUB_SHA" + - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: coned-release-linux-amd64 + path: dist + - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: coned-release-linux-arm64 + path: dist + - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: coned-release-darwin-amd64 + path: dist + - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: coned-release-darwin-arm64 + path: dist + - name: Validate exact archive set, contents, and metadata env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - name: Attest release archives - uses: actions/attest-build-provenance@0f67c3f4856b2e3261c31976d6725780e5e4c373 # v4.1.1 + RELEASE_TAG: ${{ github.ref_name }} + RELEASE_SHA: ${{ github.sha }} + shell: bash + run: | + set -euo pipefail + shopt -s nullglob + version="${RELEASE_TAG#v}" + expected_names=( + "coned-cli_${version}_linux_amd64.tar.gz" + "coned-cli_${version}_linux_arm64.tar.gz" + "coned-cli_${version}_darwin_amd64.tar.gz" + "coned-cli_${version}_darwin_arm64.tar.gz" + ) + expected_paths=() + for name in "${expected_names[@]}"; do + expected_paths+=("dist/$name") + done + mapfile -t actual_paths < <(printf '%s\n' dist/* | LC_ALL=C sort) + mapfile -t sorted_expected_paths < <(printf '%s\n' "${expected_paths[@]}" | LC_ALL=C sort) + test "${#actual_paths[@]}" -eq 4 + test "${actual_paths[*]}" = "${sorted_expected_paths[*]}" + + expected_entries=(coned LICENSE NOTICE README.md) + while IFS= read -r license; do + expected_entries+=("$license") + done < <(find third_party_licenses -type f -print | LC_ALL=C sort) + mapfile -t sorted_expected_entries < <(printf '%s\n' "${expected_entries[@]}" | LC_ALL=C sort) + expected_commit="$RELEASE_SHA" + # GoReleaser .CommitTimestamp and internal/build.Date are Unix seconds. + expected_timestamp="$(git show -s --format=%ct "$RELEASE_TAG^{commit}")" + for archive in "${actual_paths[@]}"; do + mapfile -t actual_entries < <(tar -tzf "$archive" | sed 's#/$##' | LC_ALL=C sort) + test "${actual_entries[*]}" = "${sorted_expected_entries[*]}" + workdir="$(mktemp -d)" + tar -xzf "$archive" -C "$workdir" + test -f "$workdir/coned" + description="$(file -b "$workdir/coned")" + case "$archive" in + *_linux_amd64.tar.gz) [[ "$description" == *ELF* && "$description" == *x86-64* ]] ;; + *_linux_arm64.tar.gz) [[ "$description" == *ELF* && "$description" == *aarch64* ]] ;; + *_darwin_amd64.tar.gz) [[ "$description" == *Mach-O* && "$description" == *x86_64* ]] ;; + *_darwin_arm64.tar.gz) [[ "$description" == *Mach-O* && "$description" == *arm64* ]] ;; + *) echo "unexpected archive target" >&2; exit 1 ;; + esac + strings "$workdir/coned" > "$workdir/strings" + grep -Fq "$version" "$workdir/strings" + grep -Fq "$expected_commit" "$workdir/strings" + # Match the standalone internal/build.Date value, not vcs.time=... from Go build info. + grep -Fxq "$expected_timestamp" "$workdir/strings" + grep -Fxq false "$workdir/strings" + grep -Fq 'vcs.modified=false' "$workdir/strings" + rm -rf "$workdir" + done + - name: Generate exactly one checksum manifest + shell: bash + run: | + set -euo pipefail + test ! -e dist/checksums.txt + ( + cd dist + LC_ALL=C sha256sum coned-cli_*.tar.gz > checksums.txt + ) + test "$(find dist -maxdepth 1 -type f -name 'checksums.txt' | wc -l | tr -d ' ')" = 1 + ( + cd dist + sha256sum -c checksums.txt + ) + - name: Attest all release subjects + uses: actions/attest@508db95dd578ae2727ebd6217d5ba78e4fbda05d # v4 with: subject-path: | dist/*.tar.gz dist/checksums.txt + - name: Publish exactly one GitHub release + env: + GH_TOKEN: ${{ github.token }} + RELEASE_TAG: ${{ github.ref_name }} + shell: bash + run: | + set -euo pipefail + release_args=(--verify-tag --title "$RELEASE_TAG" --generate-notes) + if [[ "$RELEASE_TAG" == *-* ]]; then + release_args+=(--prerelease) + fi + gh release create "$RELEASE_TAG" "${release_args[@]}" dist/*.tar.gz dist/checksums.txt diff --git a/.goreleaser.darwin-amd64.yml b/.goreleaser.darwin-amd64.yml new file mode 100644 index 0000000..3db1ee4 --- /dev/null +++ b/.goreleaser.darwin-amd64.yml @@ -0,0 +1,36 @@ +version: 2 +project_name: coned-cli +builds: + - id: coned + main: ./cmd/coned + binary: coned + env: [CGO_ENABLED=1] + goos: [darwin] + goarch: [amd64] + mod_timestamp: "{{ .CommitTimestamp }}" + flags: [-trimpath] + ldflags: + - -s -w + - -X github.com/zzwong/coned-cli/internal/build.Version={{.Version}} + - -X github.com/zzwong/coned-cli/internal/build.Commit={{.Commit}} + - -X github.com/zzwong/coned-cli/internal/build.Date={{.CommitTimestamp}} + - -X github.com/zzwong/coned-cli/internal/build.Dirty=false +archives: + - formats: [tar.gz] + name_template: >- + {{ .ProjectName }}_{{ .Version }}_{{ .Os }}_{{ .Arch }} + files: + - LICENSE + - NOTICE + - README.md + - third_party_licenses/* +checksum: + disable: true +changelog: + sort: asc + filters: + exclude: + - '^docs:' + - '^test:' + - '^chore:' + - '^ci:' diff --git a/.goreleaser.darwin-arm64.yml b/.goreleaser.darwin-arm64.yml new file mode 100644 index 0000000..46f9a28 --- /dev/null +++ b/.goreleaser.darwin-arm64.yml @@ -0,0 +1,36 @@ +version: 2 +project_name: coned-cli +builds: + - id: coned + main: ./cmd/coned + binary: coned + env: [CGO_ENABLED=1] + goos: [darwin] + goarch: [arm64] + mod_timestamp: "{{ .CommitTimestamp }}" + flags: [-trimpath] + ldflags: + - -s -w + - -X github.com/zzwong/coned-cli/internal/build.Version={{.Version}} + - -X github.com/zzwong/coned-cli/internal/build.Commit={{.Commit}} + - -X github.com/zzwong/coned-cli/internal/build.Date={{.CommitTimestamp}} + - -X github.com/zzwong/coned-cli/internal/build.Dirty=false +archives: + - formats: [tar.gz] + name_template: >- + {{ .ProjectName }}_{{ .Version }}_{{ .Os }}_{{ .Arch }} + files: + - LICENSE + - NOTICE + - README.md + - third_party_licenses/* +checksum: + disable: true +changelog: + sort: asc + filters: + exclude: + - '^docs:' + - '^test:' + - '^chore:' + - '^ci:' diff --git a/.goreleaser.yml b/.goreleaser.yml index 391a4af..31b41dd 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -5,7 +5,7 @@ builds: main: ./cmd/coned binary: coned env: [CGO_ENABLED=0] - goos: [linux, darwin] + goos: [linux] goarch: [amd64, arm64] mod_timestamp: "{{ .CommitTimestamp }}" flags: [-trimpath] @@ -13,7 +13,7 @@ builds: - -s -w - -X github.com/zzwong/coned-cli/internal/build.Version={{.Version}} - -X github.com/zzwong/coned-cli/internal/build.Commit={{.Commit}} - - -X github.com/zzwong/coned-cli/internal/build.Date={{.Date}} + - -X github.com/zzwong/coned-cli/internal/build.Date={{.CommitTimestamp}} - -X github.com/zzwong/coned-cli/internal/build.Dirty=false archives: - formats: [tar.gz] @@ -25,7 +25,7 @@ archives: - README.md - third_party_licenses/* checksum: - name_template: checksums.txt + disable: true changelog: sort: asc filters: @@ -34,5 +34,3 @@ changelog: - '^test:' - '^chore:' - '^ci:' -release: - prerelease: auto diff --git a/docs/releasing.md b/docs/releasing.md index df781d4..2be407c 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -1,6 +1,6 @@ # Releasing -Releases are built by the tag-triggered GitHub Actions workflow and GoReleaser. Maintainers should not upload locally built archives to an existing release. +Releases are built by the tag-triggered GitHub Actions workflow. Linux archives are built with CGO disabled on an Ubuntu runner; Darwin amd64 and arm64 archives are built natively with CGO enabled on pinned Intel and arm64 macOS runners. Maintainers should not upload locally built archives to an existing release. ## Preflight @@ -11,11 +11,15 @@ go test -race -shuffle=on ./... go vet ./... govulncheck ./... git diff --check -goreleaser check -goreleaser release --snapshot --clean +goreleaser check --config .goreleaser.yml +goreleaser check --config .goreleaser.darwin-amd64.yml +goreleaser check --config .goreleaser.darwin-arm64.yml +goreleaser release --snapshot --clean --config .goreleaser.yml +goreleaser release --snapshot --clean --config .goreleaser.darwin-amd64.yml +goreleaser release --snapshot --clean --config .goreleaser.darwin-arm64.yml ``` -Inspect snapshot archives for the binary, `LICENSE`, `NOTICE`, `README.md`, and third-party license texts. Confirm `coned version` contains the expected build metadata. +Run the Darwin amd64 snapshot on an Intel macOS host and the Darwin arm64 snapshot on an arm64 macOS host; a cross-compiled Darwin archive does not satisfy the release invariant. Snapshot configs intentionally do not generate checksums. Inspect each archive for the `coned` binary, `LICENSE`, `NOTICE`, `README.md`, and all third-party license texts. Confirm the binary contains the expected version, commit, commit timestamp, and `dirty=false` metadata. ## Publish @@ -23,7 +27,7 @@ Inspect snapshot archives for the binary, `LICENSE`, `NOTICE`, `README.md`, and 2. Choose a semantic version. Before 1.0, incompatible CLI/schema changes increment the minor version. 3. Create and verify a signed annotated tag: `git tag -s v0.1.0 -m 'v0.1.0' && git tag -v v0.1.0`. 4. Push the tag: `git push origin v0.1.0`. -5. Verify the Release workflow, generated checksums, and GitHub build-provenance attestations before announcing the release. +5. Verify the single published release, its four exact archives, the one generated `checksums.txt`, and GitHub build-provenance attestations covering every archive and the checksum manifest before announcing the release. 6. Test `gh attestation verify --repo zzwong/coned-cli` against one downloaded archive. If a release is compromised or materially broken, remove the affected artifacts, publish a security advisory when appropriate, and issue a new version. Do not silently replace published artifacts under an existing tag. diff --git a/internal/securestore/keyring.go b/internal/securestore/keyring.go index a062123..7c8a7a9 100644 --- a/internal/securestore/keyring.go +++ b/internal/securestore/keyring.go @@ -2,42 +2,89 @@ package securestore import ( "encoding/base64" + "encoding/hex" "errors" "fmt" - - "github.com/zalando/go-keyring" + "strings" ) const serviceName = "coned-cli" +const keyringBase64Prefix = "go-keyring-base64:" + +const keyringHexPrefix = "go-keyring-encoded:" + +var errInvalidKeyringValue = errors.New("invalid secure value encoding") + +type keyringDriver interface { + Get(service, account string) ([]byte, error) + Set(service, account string, value []byte) error + Delete(service, account string) error +} + // KeyringStore stores values in the operating system keyring. -type KeyringStore struct{} +type KeyringStore struct { + driver keyringDriver +} + +func (store KeyringStore) keyringDriver() keyringDriver { + if store.driver != nil { + return store.driver + } + return newKeyringDriver() +} + +func decodeKeyringValue(value []byte) ([]byte, error) { + trimmed := strings.TrimSpace(string(value)) + if strings.HasPrefix(trimmed, keyringBase64Prefix) { + decoded, err := base64.StdEncoding.DecodeString(trimmed[len(keyringBase64Prefix):]) + if err != nil { + return nil, errInvalidKeyringValue + } + return decoded, nil + } + if strings.HasPrefix(trimmed, keyringHexPrefix) { + decoded, err := hex.DecodeString(trimmed[len(keyringHexPrefix):]) + if err != nil { + return nil, errInvalidKeyringValue + } + return decoded, nil + } + return []byte(trimmed), nil +} + +func encodeKeyringValue(value []byte) []byte { + encoded := make([]byte, len(keyringBase64Prefix)+base64.StdEncoding.EncodedLen(len(value))) + copy(encoded, keyringBase64Prefix) + base64.StdEncoding.Encode(encoded[len(keyringBase64Prefix):], value) + return encoded +} func account(profile, key string) string { encoding := base64.RawURLEncoding return encoding.EncodeToString([]byte(profile)) + "/" + encoding.EncodeToString([]byte(key)) } -func (KeyringStore) Get(profile, key string) ([]byte, error) { - value, err := keyring.Get(serviceName, account(profile, key)) - if errors.Is(err, keyring.ErrNotFound) { +func (store KeyringStore) Get(profile, key string) ([]byte, error) { + value, err := store.keyringDriver().Get(serviceName, account(profile, key)) + if errors.Is(err, ErrNotFound) { return nil, ErrNotFound } if err != nil { return nil, fmt.Errorf("get secure value: %w", err) } - return []byte(value), nil + return value, nil } -func (KeyringStore) Set(profile, key string, value []byte) error { - if err := keyring.Set(serviceName, account(profile, key), string(value)); err != nil { +func (store KeyringStore) Set(profile, key string, value []byte) error { + if err := store.keyringDriver().Set(serviceName, account(profile, key), value); err != nil { return fmt.Errorf("set secure value: %w", err) } return nil } -func (KeyringStore) Delete(profile, key string) error { - if err := keyring.Delete(serviceName, account(profile, key)); errors.Is(err, keyring.ErrNotFound) { +func (store KeyringStore) Delete(profile, key string) error { + if err := store.keyringDriver().Delete(serviceName, account(profile, key)); errors.Is(err, ErrNotFound) { return ErrNotFound } else if err != nil { return fmt.Errorf("delete secure value: %w", err) diff --git a/internal/securestore/keyring_codec_test.go b/internal/securestore/keyring_codec_test.go new file mode 100644 index 0000000..e55475f --- /dev/null +++ b/internal/securestore/keyring_codec_test.go @@ -0,0 +1,80 @@ +package securestore + +import ( + "bytes" + "testing" +) + +func TestDecodeKeyringValueTrimsRawLegacyValue(t *testing.T) { + got, err := decodeKeyringValue([]byte(" \nlegacy value\t ")) + if err != nil { + t.Fatal(err) + } + if want := []byte("legacy value"); !bytes.Equal(got, want) { + t.Fatalf("decodeKeyringValue() = %q, want %q", got, want) + } +} + +func TestDecodeKeyringValueReadsStandardBase64CompatibilityValue(t *testing.T) { + got, err := decodeKeyringValue([]byte("go-keyring-base64:AAH/")) + if err != nil { + t.Fatal(err) + } + if want := []byte{0, 1, 0xff}; !bytes.Equal(got, want) { + t.Fatalf("decodeKeyringValue() = %x, want %x", got, want) + } +} + +func TestDecodeKeyringValueRejectsMalformedBase64WithoutLeakingStoredValue(t *testing.T) { + stored := []byte("go-keyring-base64:not-valid-secret") + _, err := decodeKeyringValue(stored) + if err == nil { + t.Fatal("decodeKeyringValue() error = nil, want malformed encoding error") + } + if bytes.Contains([]byte(err.Error()), stored) { + t.Fatalf("decodeKeyringValue() error exposes stored value: %q", err) + } +} + +func TestDecodeKeyringValueReadsLegacyHexCompatibilityValue(t *testing.T) { + got, err := decodeKeyringValue([]byte("go-keyring-encoded:0001ff")) + if err != nil { + t.Fatal(err) + } + if want := []byte{0, 1, 0xff}; !bytes.Equal(got, want) { + t.Fatalf("decodeKeyringValue() = %x, want %x", got, want) + } +} + +func TestDecodeKeyringValueRejectsMalformedHexWithoutLeakingStoredValue(t *testing.T) { + stored := []byte("go-keyring-encoded:not-valid-secret") + _, err := decodeKeyringValue(stored) + if err == nil { + t.Fatal("decodeKeyringValue() error = nil, want malformed encoding error") + } + if bytes.Contains([]byte(err.Error()), stored) { + t.Fatalf("decodeKeyringValue() error exposes stored value: %q", err) + } +} + +func TestEncodeKeyringValueUsesStandardBase64(t *testing.T) { + got := encodeKeyringValue([]byte{0, 1, 0xff}) + if want := []byte("go-keyring-base64:AAH/"); !bytes.Equal(got, want) { + t.Fatalf("encodeKeyringValue() = %q, want %q", got, want) + } +} + +func TestKeyringValueCodecRoundTripsBinaryLargePayload(t *testing.T) { + value := make([]byte, 16*1024) + for i := range value { + value[i] = byte((i*37 + 11) % 256) + } + + decoded, err := decodeKeyringValue(encodeKeyringValue(value)) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(decoded, value) { + t.Fatalf("codec round trip changed binary payload: got %d bytes, want %d", len(decoded), len(value)) + } +} diff --git a/internal/securestore/keyring_darwin.go b/internal/securestore/keyring_darwin.go new file mode 100644 index 0000000..1929bf9 --- /dev/null +++ b/internal/securestore/keyring_darwin.go @@ -0,0 +1,372 @@ +//go:build darwin && cgo + +package securestore + +/* +#cgo darwin LDFLAGS: -framework Security -framework CoreFoundation +#include +#include +#include +#include + +static CFStringRef coned_keychain_string(const char *value) { + return CFStringCreateWithCString(kCFAllocatorDefault, value, kCFStringEncodingUTF8); +} + +static CFDataRef coned_keychain_data(const unsigned char *value, size_t length) { + static const unsigned char empty = 0; + if (length == 0 && value == NULL) { + value = ∅ + } + return CFDataCreate(kCFAllocatorDefault, value, (CFIndex)length); +} + +static OSStatus coned_sec_item_copy_matching(const char *service, const char *account, unsigned char **out_data, size_t *out_length) { + CFStringRef service_ref = NULL; + CFStringRef account_ref = NULL; + CFDictionaryRef query = NULL; + CFTypeRef result = NULL; + OSStatus status = errSecParam; + + *out_data = NULL; + *out_length = 0; + service_ref = coned_keychain_string(service); + account_ref = coned_keychain_string(account); + if (service_ref == NULL || account_ref == NULL) { + goto cleanup; + } + + const void *keys[] = { + kSecClass, + kSecAttrService, + kSecAttrAccount, + kSecReturnData, + kSecMatchLimit, + }; + const void *values[] = { + kSecClassGenericPassword, + service_ref, + account_ref, + kCFBooleanTrue, + kSecMatchLimitOne, + }; + query = CFDictionaryCreate(kCFAllocatorDefault, keys, values, sizeof(keys) / sizeof(keys[0]), &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks); + if (query == NULL) { + goto cleanup; + } + + status = SecItemCopyMatching(query, &result); + if (status == errSecSuccess) { + if (result == NULL || CFGetTypeID(result) != CFDataGetTypeID()) { + status = errSecParam; + goto cleanup; + } + + CFDataRef data = (CFDataRef)result; + CFIndex length = CFDataGetLength(data); + const UInt8 *bytes = CFDataGetBytePtr(data); + if (length < 0 || (length > 0 && bytes == NULL)) { + status = errSecParam; + goto cleanup; + } + if (length > 0) { + *out_data = (unsigned char *)malloc((size_t)length); + if (*out_data == NULL) { + status = errSecParam; + goto cleanup; + } + memcpy(*out_data, bytes, (size_t)length); + } + *out_length = (size_t)length; + } + +cleanup: + if (result != NULL) { + CFRelease(result); + } + if (query != NULL) { + CFRelease(query); + } + if (account_ref != NULL) { + CFRelease(account_ref); + } + if (service_ref != NULL) { + CFRelease(service_ref); + } + return status; +} + +static OSStatus coned_sec_item_update(const char *service, const char *account, const unsigned char *value, size_t value_length) { + CFStringRef service_ref = NULL; + CFStringRef account_ref = NULL; + CFDataRef value_ref = NULL; + CFDictionaryRef query = NULL; + CFDictionaryRef attributes = NULL; + OSStatus status = errSecParam; + + service_ref = coned_keychain_string(service); + account_ref = coned_keychain_string(account); + value_ref = coned_keychain_data(value, value_length); + if (service_ref == NULL || account_ref == NULL || value_ref == NULL) { + goto cleanup; + } + + const void *query_keys[] = {kSecClass, kSecAttrService, kSecAttrAccount}; + const void *query_values[] = {kSecClassGenericPassword, service_ref, account_ref}; + query = CFDictionaryCreate(kCFAllocatorDefault, query_keys, query_values, sizeof(query_keys) / sizeof(query_keys[0]), &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks); + if (query == NULL) { + goto cleanup; + } + + const void *attribute_keys[] = {kSecValueData}; + const void *attribute_values[] = {value_ref}; + attributes = CFDictionaryCreate(kCFAllocatorDefault, attribute_keys, attribute_values, sizeof(attribute_keys) / sizeof(attribute_keys[0]), &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks); + if (attributes == NULL) { + goto cleanup; + } + + status = SecItemUpdate(query, attributes); + +cleanup: + if (attributes != NULL) { + CFRelease(attributes); + } + if (query != NULL) { + CFRelease(query); + } + if (value_ref != NULL) { + CFRelease(value_ref); + } + if (account_ref != NULL) { + CFRelease(account_ref); + } + if (service_ref != NULL) { + CFRelease(service_ref); + } + return status; +} + +static OSStatus coned_sec_item_add(const char *service, const char *account, const unsigned char *value, size_t value_length) { + CFStringRef service_ref = NULL; + CFStringRef account_ref = NULL; + CFDataRef value_ref = NULL; + CFDictionaryRef attributes = NULL; + OSStatus status = errSecParam; + + service_ref = coned_keychain_string(service); + account_ref = coned_keychain_string(account); + value_ref = coned_keychain_data(value, value_length); + if (service_ref == NULL || account_ref == NULL || value_ref == NULL) { + goto cleanup; + } + + const void *keys[] = {kSecClass, kSecAttrService, kSecAttrAccount, kSecValueData}; + const void *values[] = {kSecClassGenericPassword, service_ref, account_ref, value_ref}; + attributes = CFDictionaryCreate(kCFAllocatorDefault, keys, values, sizeof(keys) / sizeof(keys[0]), &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks); + if (attributes == NULL) { + goto cleanup; + } + + status = SecItemAdd(attributes, NULL); + +cleanup: + if (attributes != NULL) { + CFRelease(attributes); + } + if (value_ref != NULL) { + CFRelease(value_ref); + } + if (account_ref != NULL) { + CFRelease(account_ref); + } + if (service_ref != NULL) { + CFRelease(service_ref); + } + return status; +} + +static OSStatus coned_sec_item_delete(const char *service, const char *account) { + CFStringRef service_ref = NULL; + CFStringRef account_ref = NULL; + CFDictionaryRef query = NULL; + OSStatus status = errSecParam; + + service_ref = coned_keychain_string(service); + account_ref = coned_keychain_string(account); + if (service_ref == NULL || account_ref == NULL) { + goto cleanup; + } + + const void *keys[] = {kSecClass, kSecAttrService, kSecAttrAccount}; + const void *values[] = {kSecClassGenericPassword, service_ref, account_ref}; + query = CFDictionaryCreate(kCFAllocatorDefault, keys, values, sizeof(keys) / sizeof(keys[0]), &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks); + if (query == NULL) { + goto cleanup; + } + + status = SecItemDelete(query); + +cleanup: + if (query != NULL) { + CFRelease(query); + } + if (account_ref != NULL) { + CFRelease(account_ref); + } + if (service_ref != NULL) { + CFRelease(service_ref); + } + return status; +} +*/ +import "C" + +import ( + "errors" + "unsafe" +) + +const ( + darwinSecItemSuccess int32 = 0 + darwinSecItemNotFound int32 = -25300 + darwinSecItemDuplicate int32 = -25299 +) + +var errDarwinKeyringOperation = errors.New("secure keychain operation failed") + +type darwinSecItemResult struct { + data []byte + release func() +} + +type darwinSecItemOps interface { + copyMatching(service, account string) (darwinSecItemResult, int32) + update(service, account string, value []byte) int32 + add(service, account string, value []byte) int32 + delete(service, account string) int32 +} + +type darwinKeyringDriver struct { + ops darwinSecItemOps +} + +func newKeyringDriver() keyringDriver { + return darwinKeyringDriver{ops: darwinSecItemCgoOps{}} +} + +func (driver darwinKeyringDriver) operations() darwinSecItemOps { + if driver.ops != nil { + return driver.ops + } + return darwinSecItemCgoOps{} +} + +func (driver darwinKeyringDriver) Get(service, account string) ([]byte, error) { + result, status := driver.operations().copyMatching(service, account) + if result.release != nil { + defer result.release() + } + if err := reduceDarwinSecItemStatus(status); err != nil { + return nil, err + } + decoded, err := decodeKeyringValue(result.data) + if err != nil { + return nil, err + } + return append([]byte(nil), decoded...), nil +} + +func (driver darwinKeyringDriver) Set(service, account string, value []byte) error { + encoded := encodeKeyringValue(value) + operations := driver.operations() + status := operations.update(service, account, encoded) + if status == darwinSecItemSuccess { + return nil + } + if status != darwinSecItemNotFound { + return reduceDarwinSecItemStatus(status) + } + + status = operations.add(service, account, encoded) + if status == darwinSecItemSuccess { + return nil + } + if status != darwinSecItemDuplicate { + return reduceDarwinSecItemStatus(status) + } + + return reduceDarwinSecItemStatus(operations.update(service, account, encoded)) +} + +func (driver darwinKeyringDriver) Delete(service, account string) error { + return reduceDarwinSecItemStatus(driver.operations().delete(service, account)) +} + +func reduceDarwinSecItemStatus(status int32) error { + switch status { + case darwinSecItemSuccess: + return nil + case darwinSecItemNotFound: + return ErrNotFound + default: + return errDarwinKeyringOperation + } +} + +type darwinSecItemCgoOps struct{} + +func (darwinSecItemCgoOps) copyMatching(service, account string) (darwinSecItemResult, int32) { + cService := C.CString(service) + defer C.free(unsafe.Pointer(cService)) + cAccount := C.CString(account) + defer C.free(unsafe.Pointer(cAccount)) + + var data *C.uchar + var length C.size_t + status := C.coned_sec_item_copy_matching(cService, cAccount, &data, &length) + if data == nil { + return darwinSecItemResult{}, int32(status) + } + value := append([]byte(nil), unsafe.Slice((*byte)(unsafe.Pointer(data)), int(length))...) + C.free(unsafe.Pointer(data)) + return darwinSecItemResult{data: value}, int32(status) +} + +func (darwinSecItemCgoOps) update(service, account string, value []byte) int32 { + cService := C.CString(service) + defer C.free(unsafe.Pointer(cService)) + cAccount := C.CString(account) + defer C.free(unsafe.Pointer(cAccount)) + cValue, valueLength := cKeyringBytes(value) + if cValue != nil { + defer C.free(cValue) + } + return int32(C.coned_sec_item_update(cService, cAccount, (*C.uchar)(cValue), valueLength)) +} + +func (darwinSecItemCgoOps) add(service, account string, value []byte) int32 { + cService := C.CString(service) + defer C.free(unsafe.Pointer(cService)) + cAccount := C.CString(account) + defer C.free(unsafe.Pointer(cAccount)) + cValue, valueLength := cKeyringBytes(value) + if cValue != nil { + defer C.free(cValue) + } + return int32(C.coned_sec_item_add(cService, cAccount, (*C.uchar)(cValue), valueLength)) +} + +func (darwinSecItemCgoOps) delete(service, account string) int32 { + cService := C.CString(service) + defer C.free(unsafe.Pointer(cService)) + cAccount := C.CString(account) + defer C.free(unsafe.Pointer(cAccount)) + return int32(C.coned_sec_item_delete(cService, cAccount)) +} + +func cKeyringBytes(value []byte) (unsafe.Pointer, C.size_t) { + if len(value) == 0 { + return nil, 0 + } + return C.CBytes(value), C.size_t(len(value)) +} diff --git a/internal/securestore/keyring_darwin_integration_test.go b/internal/securestore/keyring_darwin_integration_test.go new file mode 100644 index 0000000..fc219ed --- /dev/null +++ b/internal/securestore/keyring_darwin_integration_test.go @@ -0,0 +1,214 @@ +//go:build darwin && cgo + +package securestore + +import ( + "bytes" + "errors" + "fmt" + "os" + "os/exec" + "strings" + "sync/atomic" + "testing" + "time" +) + +const ( + keychainIntegrationEnv = "CONED_KEYCHAIN_TEST" + keychainIntegrationChildEnv = "CONED_KEYCHAIN_TEST_CHILD" + keychainIntegrationProfileEnv = "CONED_KEYCHAIN_TEST_PROFILE" + keychainIntegrationKeyEnv = "CONED_KEYCHAIN_TEST_KEY" +) + +var keychainIntegrationCounter uint64 + +type keychainIntegrationItem struct { + store KeyringStore + profile string + key string + account string +} + +func requireKeychainIntegration(t *testing.T) { + t.Helper() + if os.Getenv(keychainIntegrationEnv) != "1" { + t.Skip("set CONED_KEYCHAIN_TEST=1 to run live macOS Keychain integration tests") + } +} + +func newKeychainIntegrationItem(t *testing.T) keychainIntegrationItem { + t.Helper() + suffix := fmt.Sprintf("%d-%d-%d", os.Getpid(), time.Now().UnixNano(), atomic.AddUint64(&keychainIntegrationCounter, 1)) + return registerKeychainIntegrationItem(t, + "coned-keychain-integration-profile-"+suffix, + "coned-keychain-integration-key-"+suffix, + ) +} + +func newKeychainIntegrationChildItem(t *testing.T) keychainIntegrationItem { + t.Helper() + profile := os.Getenv(keychainIntegrationProfileEnv) + key := os.Getenv(keychainIntegrationKeyEnv) + if profile == "" || key == "" { + t.Fatalf("child test missing synthetic Keychain identity") + } + return registerKeychainIntegrationItem(t, profile, key) +} + +func registerKeychainIntegrationItem(t *testing.T, profile, key string) keychainIntegrationItem { + t.Helper() + item := keychainIntegrationItem{ + store: KeyringStore{}, + profile: profile, + key: key, + account: account(profile, key), + } + t.Cleanup(func() { + if err := item.store.Delete(item.profile, item.key); err != nil && !errors.Is(err, ErrNotFound) { + t.Errorf("cleanup Delete() failed: %v", err) + } + assertExactKeychainItemMissing(t, item.account) + }) + return item +} + +func keychainIntegrationChildEnvironment(item keychainIntegrationItem) []string { + env := append([]string(nil), os.Environ()...) + env = withEnvironmentValue(env, keychainIntegrationEnv, "1") + env = withEnvironmentValue(env, keychainIntegrationChildEnv, "1") + env = withEnvironmentValue(env, keychainIntegrationProfileEnv, item.profile) + env = withEnvironmentValue(env, keychainIntegrationKeyEnv, item.key) + return env +} + +func withEnvironmentValue(env []string, name, value string) []string { + prefix := name + "=" + for i, entry := range env { + if strings.HasPrefix(entry, prefix) { + env[i] = prefix + value + return env + } + } + return append(env, prefix+value) +} + +func assertExactKeychainItemMissing(t *testing.T, accountName string) { + t.Helper() + result, status := (darwinSecItemCgoOps{}).copyMatching(serviceName, accountName) + if result.release != nil { + result.release() + } + if status != darwinSecItemNotFound { + t.Fatalf("exact Keychain lookup for service %q/account %q status = %d, want item-not-found", serviceName, accountName, status) + } + t.Logf("exact Keychain cleanup verified for service %q/account %q", serviceName, accountName) +} + +func TestDarwinKeyringIntegrationMissingItem(t *testing.T) { + requireKeychainIntegration(t) + item := newKeychainIntegrationItem(t) + + if _, err := item.store.Get(item.profile, item.key); err != ErrNotFound { + t.Fatalf("Get() error = %v, want ErrNotFound", err) + } +} + +func TestDarwinKeyringIntegrationSmallRoundTrip(t *testing.T) { + requireKeychainIntegration(t) + item := newKeychainIntegrationItem(t) + want := []byte("synthetic-small-keychain-value") + + if err := item.store.Set(item.profile, item.key, want); err != nil { + t.Fatalf("Set() failed: %v", err) + } + got, err := item.store.Get(item.profile, item.key) + if err != nil { + t.Fatalf("Get() failed: %v", err) + } + if !bytes.Equal(got, want) { + t.Fatalf("Get() = %q, want %q", got, want) + } +} + +func TestDarwinKeyringIntegrationLargeBinaryRoundTrip(t *testing.T) { + requireKeychainIntegration(t) + item := newKeychainIntegrationItem(t) + want := make([]byte, 16*1024) + for i := range want { + want[i] = byte((i*29 + 7) % 256) + } + + if err := item.store.Set(item.profile, item.key, want); err != nil { + t.Fatalf("Set() failed: %v", err) + } + got, err := item.store.Get(item.profile, item.key) + if err != nil { + t.Fatalf("Get() failed: %v", err) + } + if !bytes.Equal(got, want) { + t.Fatalf("large binary round trip changed payload: got %d bytes, want %d", len(got), len(want)) + } +} + +func TestDarwinKeyringIntegrationOverwrite(t *testing.T) { + requireKeychainIntegration(t) + item := newKeychainIntegrationItem(t) + first := []byte("synthetic-first-value") + second := []byte("synthetic-overwritten-value") + + if err := item.store.Set(item.profile, item.key, first); err != nil { + t.Fatalf("first Set() failed: %v", err) + } + if err := item.store.Set(item.profile, item.key, second); err != nil { + t.Fatalf("second Set() failed: %v", err) + } + got, err := item.store.Get(item.profile, item.key) + if err != nil { + t.Fatalf("Get() failed: %v", err) + } + if !bytes.Equal(got, second) { + t.Fatalf("Get() after overwrite = %q, want %q", got, second) + } +} + +func TestDarwinKeyringIntegrationDeleteAndRepeatedDelete(t *testing.T) { + requireKeychainIntegration(t) + item := newKeychainIntegrationItem(t) + + if err := item.store.Set(item.profile, item.key, []byte("synthetic-delete-value")); err != nil { + t.Fatalf("Set() failed: %v", err) + } + if err := item.store.Delete(item.profile, item.key); err != nil { + t.Fatalf("Delete() failed: %v", err) + } + if _, err := item.store.Get(item.profile, item.key); err != ErrNotFound { + t.Fatalf("Get() after Delete() error = %v, want ErrNotFound", err) + } + if err := item.store.Delete(item.profile, item.key); err != ErrNotFound { + t.Fatalf("repeated Delete() error = %v, want ErrNotFound", err) + } +} + +func TestDarwinKeyringIntegrationCleanupAfterForcedFailure(t *testing.T) { + requireKeychainIntegration(t) + if os.Getenv(keychainIntegrationChildEnv) == "1" { + item := newKeychainIntegrationChildItem(t) + if err := item.store.Set(item.profile, item.key, []byte("synthetic-forced-failure-value")); err != nil { + t.Fatalf("child Set() failed: %v", err) + } + t.Fatal("synthetic keychain assertion failure") + } + + item := newKeychainIntegrationItem(t) + cmd := exec.Command(os.Args[0], "-test.run", "^TestDarwinKeyringIntegrationCleanupAfterForcedFailure$") + cmd.Env = keychainIntegrationChildEnvironment(item) + output, err := cmd.CombinedOutput() + if err == nil { + t.Fatal("child unexpectedly passed instead of exercising forced failure cleanup") + } + if !bytes.Contains(output, []byte("synthetic keychain assertion failure")) { + t.Fatalf("child output did not prove forced assertion ran: %q", output) + } + assertExactKeychainItemMissing(t, item.account) +} diff --git a/internal/securestore/keyring_darwin_nocgo.go b/internal/securestore/keyring_darwin_nocgo.go new file mode 100644 index 0000000..586948b --- /dev/null +++ b/internal/securestore/keyring_darwin_nocgo.go @@ -0,0 +1,25 @@ +//go:build darwin && !cgo + +package securestore + +import "errors" + +var errKeyringUnavailable = errors.New("secure keyring unavailable") + +type darwinNoCGOKeyringDriver struct{} + +func newKeyringDriver() keyringDriver { + return darwinNoCGOKeyringDriver{} +} + +func (darwinNoCGOKeyringDriver) Get(string, string) ([]byte, error) { + return nil, errKeyringUnavailable +} + +func (darwinNoCGOKeyringDriver) Set(string, string, []byte) error { + return errKeyringUnavailable +} + +func (darwinNoCGOKeyringDriver) Delete(string, string) error { + return errKeyringUnavailable +} diff --git a/internal/securestore/keyring_darwin_nocgo_test.go b/internal/securestore/keyring_darwin_nocgo_test.go new file mode 100644 index 0000000..4bedb31 --- /dev/null +++ b/internal/securestore/keyring_darwin_nocgo_test.go @@ -0,0 +1,24 @@ +//go:build darwin && !cgo + +package securestore + +import ( + "errors" + "strings" + "testing" +) + +func TestDarwinNoCGODriverFailsClosedWithoutSecretData(t *testing.T) { + const secret = "synthetic-secret" + driver := newKeyringDriver() + + if _, err := driver.Get("coned-cli", secret); err == nil || errors.Is(err, ErrNotFound) || strings.Contains(err.Error(), secret) { + t.Fatalf("Get() error = %v, want sanitized unavailable error", err) + } + if err := driver.Set("coned-cli", secret, []byte(secret)); err == nil || strings.Contains(err.Error(), secret) { + t.Fatalf("Set() error = %v, want sanitized unavailable error", err) + } + if err := driver.Delete("coned-cli", secret); err == nil || strings.Contains(err.Error(), secret) { + t.Fatalf("Delete() error = %v, want sanitized unavailable error", err) + } +} diff --git a/internal/securestore/keyring_darwin_test.go b/internal/securestore/keyring_darwin_test.go new file mode 100644 index 0000000..ea26fd0 --- /dev/null +++ b/internal/securestore/keyring_darwin_test.go @@ -0,0 +1,266 @@ +//go:build darwin && cgo + +package securestore + +import ( + "bytes" + "errors" + "strings" + "testing" +) + +type fakeDarwinSecItemOps struct { + copyStatus int32 + copyResult darwinSecItemResult + updateStatus []int32 + addStatus []int32 + deleteStatus int32 + + copyService, copyAccount string + updateService, updateAccount string + addService, addAccount string + deleteService, deleteAccount string + updateValues [][]byte + addValues [][]byte +} + +func (f *fakeDarwinSecItemOps) copyMatching(service, account string) (darwinSecItemResult, int32) { + f.copyService, f.copyAccount = service, account + return f.copyResult, f.copyStatus +} + +func (f *fakeDarwinSecItemOps) update(service, account string, value []byte) int32 { + f.updateService, f.updateAccount = service, account + f.updateValues = append(f.updateValues, append([]byte(nil), value...)) + return popDarwinStatus(&f.updateStatus) +} + +func (f *fakeDarwinSecItemOps) add(service, account string, value []byte) int32 { + f.addService, f.addAccount = service, account + f.addValues = append(f.addValues, append([]byte(nil), value...)) + return popDarwinStatus(&f.addStatus) +} + +func (f *fakeDarwinSecItemOps) delete(service, account string) int32 { + f.deleteService, f.deleteAccount = service, account + return f.deleteStatus +} + +func popDarwinStatus(statuses *[]int32) int32 { + if len(*statuses) == 0 { + return darwinSecItemSuccess + } + status := (*statuses)[0] + *statuses = (*statuses)[1:] + return status +} + +func TestDarwinKeyringDriverGetCopiesResultAndReleasesIt(t *testing.T) { + releases := 0 + fake := &fakeDarwinSecItemOps{ + copyStatus: darwinSecItemSuccess, + copyResult: darwinSecItemResult{ + data: []byte{0, 1, 0xff}, + release: func() { releases++ }, + }, + } + driver := darwinKeyringDriver{ops: fake} + + got, err := driver.Get("coned-cli", "account") + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(got, []byte{0, 1, 0xff}) { + t.Fatalf("Get() = %x, want 0001ff", got) + } + if releases != 1 { + t.Fatalf("returned data releases = %d, want 1", releases) + } + if fake.copyService != "coned-cli" || fake.copyAccount != "account" { + t.Fatalf("copy identifiers = %q/%q, want coned-cli/account", fake.copyService, fake.copyAccount) + } +} + +func TestDarwinKeyringDriverGetDecodesCompatibilityValue(t *testing.T) { + fake := &fakeDarwinSecItemOps{ + copyStatus: darwinSecItemSuccess, + copyResult: darwinSecItemResult{data: []byte("go-keyring-base64:AAH/")}, + } + driver := darwinKeyringDriver{ops: fake} + + got, err := driver.Get("service", "account") + if err != nil { + t.Fatal(err) + } + if want := []byte{0, 1, 0xff}; !bytes.Equal(got, want) { + t.Fatalf("Get() = %x, want %x", got, want) + } +} + +func TestDarwinKeyringDriverGetRejectsMalformedCompatibilityValue(t *testing.T) { + const stored = "go-keyring-base64:not-valid-secret" + fake := &fakeDarwinSecItemOps{ + copyStatus: darwinSecItemSuccess, + copyResult: darwinSecItemResult{data: []byte(stored)}, + } + driver := darwinKeyringDriver{ops: fake} + + _, err := driver.Get("service", "account") + if err == nil || strings.Contains(err.Error(), stored) { + t.Fatalf("Get() error = %v, want sanitized malformed-value error", err) + } +} + +func TestDarwinKeyringDriverGetMapsNotFound(t *testing.T) { + fake := &fakeDarwinSecItemOps{copyStatus: darwinSecItemNotFound} + driver := darwinKeyringDriver{ops: fake} + + if _, err := driver.Get("coned-cli", "missing"); !errors.Is(err, ErrNotFound) { + t.Fatalf("Get() error = %v, want ErrNotFound", err) + } +} + +func TestDarwinKeyringDriverGetSanitizesOSStatusFailure(t *testing.T) { + fake := &fakeDarwinSecItemOps{copyStatus: -34018} + driver := darwinKeyringDriver{ops: fake} + + _, err := driver.Get("coned-cli", "synthetic-secret-account") + if err == nil || strings.Contains(err.Error(), "synthetic-secret-account") { + t.Fatalf("Get() error = %v, want sanitized failure", err) + } +} + +func TestDarwinKeyringDriverReleasesCopyResultOnEveryStatus(t *testing.T) { + for _, test := range []struct { + name string + status int32 + }{ + {name: "success", status: darwinSecItemSuccess}, + {name: "not found", status: darwinSecItemNotFound}, + {name: "failure", status: -34018}, + } { + t.Run(test.name, func(t *testing.T) { + releases := 0 + fake := &fakeDarwinSecItemOps{ + copyStatus: test.status, + copyResult: darwinSecItemResult{release: func() { releases++ }}, + } + _, _ = (darwinKeyringDriver{ops: fake}).Get("service", "account") + if releases != 1 { + t.Fatalf("returned data releases = %d, want 1", releases) + } + }) + } +} + +func TestDarwinKeyringDriverSetUpdatesExistingItem(t *testing.T) { + fake := &fakeDarwinSecItemOps{updateStatus: []int32{darwinSecItemSuccess}} + driver := darwinKeyringDriver{ops: fake} + + if err := driver.Set("coned-cli", "account", []byte("synthetic-value")); err != nil { + t.Fatal(err) + } + if len(fake.updateValues) != 1 || !bytes.Equal(fake.updateValues[0], encodeKeyringValue([]byte("synthetic-value"))) { + t.Fatalf("update values = %q, want encoded value", fake.updateValues) + } + if len(fake.addValues) != 0 { + t.Fatalf("add calls = %d, want 0", len(fake.addValues)) + } +} + +func TestDarwinKeyringDriverSetAddsWhenUpdateReportsNotFound(t *testing.T) { + fake := &fakeDarwinSecItemOps{ + updateStatus: []int32{darwinSecItemNotFound}, + addStatus: []int32{darwinSecItemSuccess}, + } + driver := darwinKeyringDriver{ops: fake} + + if err := driver.Set("service", "account", []byte("value")); err != nil { + t.Fatal(err) + } + if len(fake.updateValues) != 1 || len(fake.addValues) != 1 { + t.Fatalf("update/add calls = %d/%d, want 1/1", len(fake.updateValues), len(fake.addValues)) + } + if !bytes.Equal(fake.addValues[0], fake.updateValues[0]) { + t.Fatalf("add value = %q, want same encoded value as update", fake.addValues[0]) + } +} + +func TestDarwinKeyringDriverSetRetriesUpdateOnceAfterDuplicateAdd(t *testing.T) { + fake := &fakeDarwinSecItemOps{ + updateStatus: []int32{darwinSecItemNotFound, darwinSecItemSuccess}, + addStatus: []int32{darwinSecItemDuplicate}, + } + driver := darwinKeyringDriver{ops: fake} + + if err := driver.Set("service", "account", []byte("value")); err != nil { + t.Fatal(err) + } + if len(fake.updateValues) != 2 || len(fake.addValues) != 1 { + t.Fatalf("update/add calls = %d/%d, want 2/1", len(fake.updateValues), len(fake.addValues)) + } +} + +func TestDarwinKeyringDriverSetStopsAfterTerminalUpdateFailure(t *testing.T) { + fake := &fakeDarwinSecItemOps{updateStatus: []int32{-34018}} + driver := darwinKeyringDriver{ops: fake} + + if err := driver.Set("service", "account", []byte("value")); err == nil || len(fake.addValues) != 0 { + t.Fatalf("Set() error = %v, add calls = %d; want terminal error and no add", err, len(fake.addValues)) + } +} + +func TestDarwinKeyringDriverSetStopsAfterTerminalAddFailure(t *testing.T) { + fake := &fakeDarwinSecItemOps{ + updateStatus: []int32{darwinSecItemNotFound}, + addStatus: []int32{-34018}, + } + driver := darwinKeyringDriver{ops: fake} + + if err := driver.Set("service", "account", []byte("value")); err == nil || len(fake.updateValues) != 1 || len(fake.addValues) != 1 { + t.Fatalf("Set() error = %v, update/add calls = %d/%d; want terminal add error", err, len(fake.updateValues), len(fake.addValues)) + } +} + +func TestDarwinKeyringDriverSetRetriesDuplicateRaceOnlyOnce(t *testing.T) { + fake := &fakeDarwinSecItemOps{ + updateStatus: []int32{darwinSecItemNotFound, darwinSecItemNotFound}, + addStatus: []int32{darwinSecItemDuplicate}, + } + driver := darwinKeyringDriver{ops: fake} + + if err := driver.Set("service", "account", []byte("value")); err == nil || len(fake.updateValues) != 2 || len(fake.addValues) != 1 { + t.Fatalf("Set() error = %v, update/add calls = %d/%d; want one bounded retry", err, len(fake.updateValues), len(fake.addValues)) + } +} + +func TestDarwinKeyringDriverDeleteSuccess(t *testing.T) { + fake := &fakeDarwinSecItemOps{deleteStatus: darwinSecItemSuccess} + driver := darwinKeyringDriver{ops: fake} + + if err := driver.Delete("service", "account"); err != nil { + t.Fatal(err) + } + if fake.deleteService != "service" || fake.deleteAccount != "account" { + t.Fatalf("delete identifiers = %q/%q, want service/account", fake.deleteService, fake.deleteAccount) + } +} + +func TestDarwinKeyringDriverDeleteMapsNotFound(t *testing.T) { + fake := &fakeDarwinSecItemOps{deleteStatus: darwinSecItemNotFound} + driver := darwinKeyringDriver{ops: fake} + + if err := driver.Delete("service", "missing"); !errors.Is(err, ErrNotFound) { + t.Fatalf("Delete() error = %v, want ErrNotFound", err) + } +} + +func TestDarwinKeyringDriverDeleteSanitizesOSStatusFailure(t *testing.T) { + fake := &fakeDarwinSecItemOps{deleteStatus: -34018} + driver := darwinKeyringDriver{ops: fake} + + err := driver.Delete("service", "synthetic-secret-account") + if err == nil || strings.Contains(err.Error(), "synthetic-secret-account") { + t.Fatalf("Delete() error = %v, want sanitized failure", err) + } +} diff --git a/internal/securestore/keyring_other.go b/internal/securestore/keyring_other.go new file mode 100644 index 0000000..628595a --- /dev/null +++ b/internal/securestore/keyring_other.go @@ -0,0 +1,38 @@ +//go:build !darwin + +package securestore + +import ( + "errors" + + "github.com/zalando/go-keyring" +) + +type goKeyringDriver struct{} + +func newKeyringDriver() keyringDriver { + return goKeyringDriver{} +} + +func (goKeyringDriver) Get(service, account string) ([]byte, error) { + value, err := keyring.Get(service, account) + if errors.Is(err, keyring.ErrNotFound) { + return nil, ErrNotFound + } + if err != nil { + return nil, err + } + return []byte(value), nil +} + +func (goKeyringDriver) Set(service, account string, value []byte) error { + return keyring.Set(service, account, string(value)) +} + +func (goKeyringDriver) Delete(service, account string) error { + err := keyring.Delete(service, account) + if errors.Is(err, keyring.ErrNotFound) { + return ErrNotFound + } + return err +} diff --git a/internal/securestore/store_test.go b/internal/securestore/store_test.go index 5269c29..b259f21 100644 --- a/internal/securestore/store_test.go +++ b/internal/securestore/store_test.go @@ -1,10 +1,39 @@ package securestore import ( + "bytes" "errors" + "strings" "testing" + + "github.com/zalando/go-keyring" ) +type recordingKeyringDriver struct { + getService, getAccount string + setService, setAccount string + deleteService, deleteAccount string + getValue []byte + setValue []byte + getErr, setErr, deleteErr error +} + +func (d *recordingKeyringDriver) Get(service, account string) ([]byte, error) { + d.getService, d.getAccount = service, account + return d.getValue, d.getErr +} + +func (d *recordingKeyringDriver) Set(service, account string, value []byte) error { + d.setService, d.setAccount = service, account + d.setValue = append([]byte(nil), value...) + return d.setErr +} + +func (d *recordingKeyringDriver) Delete(service, account string) error { + d.deleteService, d.deleteAccount = service, account + return d.deleteErr +} + func TestKeyringAccountEncodesComponentsWithoutCollisions(t *testing.T) { if first, second := account("a/b", "c"), account("a", "b/c"); first == second { t.Fatalf("accounts collide: %q", first) @@ -14,6 +43,105 @@ func TestKeyringAccountEncodesComponentsWithoutCollisions(t *testing.T) { } } +func TestKeyringStoreGetForwardsExactIdentifiersThroughDriver(t *testing.T) { + driver := &recordingKeyringDriver{getValue: []byte("synthetic-value")} + store := KeyringStore{driver: driver} + + got, err := store.Get("a/b", "c") + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(got, []byte("synthetic-value")) { + t.Fatalf("Get() = %q, want %q", got, "synthetic-value") + } + if driver.getService != "coned-cli" || driver.getAccount != "YS9i/Yw" { + t.Fatalf("driver identifiers = %q/%q, want %q/%q", driver.getService, driver.getAccount, "coned-cli", "YS9i/Yw") + } +} + +func TestKeyringStoreSetForwardsExactIdentifiersAndValueThroughDriver(t *testing.T) { + driver := &recordingKeyringDriver{} + store := KeyringStore{driver: driver} + value := []byte{0, 1, 0xff} + + if err := store.Set("a/b", "c", value); err != nil { + t.Fatal(err) + } + if driver.setService != "coned-cli" || driver.setAccount != "YS9i/Yw" { + t.Fatalf("driver identifiers = %q/%q, want %q/%q", driver.setService, driver.setAccount, "coned-cli", "YS9i/Yw") + } + if !bytes.Equal(driver.setValue, value) { + t.Fatalf("driver value = %x, want %x", driver.setValue, value) + } +} + +func TestKeyringStoreDeleteForwardsExactIdentifiersThroughDriver(t *testing.T) { + driver := &recordingKeyringDriver{} + store := KeyringStore{driver: driver} + + if err := store.Delete("a/b", "c"); err != nil { + t.Fatal(err) + } + if driver.deleteService != "coned-cli" || driver.deleteAccount != "YS9i/Yw" { + t.Fatalf("driver identifiers = %q/%q, want %q/%q", driver.deleteService, driver.deleteAccount, "coned-cli", "YS9i/Yw") + } +} + +func TestKeyringStoreMapsDriverNotFoundOnGet(t *testing.T) { + store := KeyringStore{driver: &recordingKeyringDriver{getErr: ErrNotFound}} + + _, err := store.Get("profile", "key") + if err != ErrNotFound { + t.Fatalf("Get() error = %v, want ErrNotFound", err) + } +} + +func TestKeyringStoreMapsDriverNotFoundOnDelete(t *testing.T) { + store := KeyringStore{driver: &recordingKeyringDriver{deleteErr: ErrNotFound}} + + if err := store.Delete("profile", "key"); err != ErrNotFound { + t.Fatalf("Delete() error = %v, want ErrNotFound", err) + } +} + +func TestKeyringStoreWrapsDriverGetErrorPreservingIdentityAndContext(t *testing.T) { + backendErr := errors.New("backend get failure") + store := KeyringStore{driver: &recordingKeyringDriver{getErr: backendErr}} + + _, err := store.Get("profile", "key") + if !errors.Is(err, backendErr) { + t.Fatalf("Get() error = %v, want errors.Is(error, backendErr)", err) + } + if !strings.Contains(err.Error(), "get secure value") { + t.Fatalf("Get() error = %v, want get operation context", err) + } +} + +func TestKeyringStoreWrapsGoKeyringSetErrorPreservingIdentityAndContext(t *testing.T) { + store := KeyringStore{driver: &recordingKeyringDriver{setErr: keyring.ErrSetDataTooBig}} + + err := store.Set("profile", "key", []byte("synthetic-value")) + if !errors.Is(err, keyring.ErrSetDataTooBig) { + t.Fatalf("Set() error = %v, want errors.Is(error, keyring.ErrSetDataTooBig)", err) + } + if !strings.Contains(err.Error(), "set secure value") { + t.Fatalf("Set() error = %v, want set operation context", err) + } +} + +func TestKeyringStoreWrapsDriverDeleteErrorPreservingIdentityAndContext(t *testing.T) { + backendErr := errors.New("backend delete failure") + store := KeyringStore{driver: &recordingKeyringDriver{deleteErr: backendErr}} + + err := store.Delete("profile", "key") + if !errors.Is(err, backendErr) { + t.Fatalf("Delete() error = %v, want errors.Is(error, backendErr)", err) + } + if !strings.Contains(err.Error(), "delete secure value") { + t.Fatalf("Delete() error = %v, want delete operation context", err) + } +} + func TestMemoryStoreProfileIsolation(t *testing.T) { store := NewMemoryStore() if err := store.Set("one", "credentials", []byte("secret-one")); err != nil {