Conversation
This fixes iOS builds
There was a problem hiding this comment.
1 issue found across 12 files
Confidence score: 4/5
- This PR is likely safe to merge, with the main risk being dependency surface area rather than a confirmed runtime bug.
- In
go.mod, switching to thebyteness/keyringfork appears to introduce many additional indirect packages (including observability/runtime SDK stacks), which can increase build, maintenance, and supply-chain exposure. - Given the reported severity (5/10) and no explicit functional breakage noted, this looks like a moderate caution item instead of a merge blocker.
- Pay close attention to
go.mod- the expanded transitive dependency graph from the keyring fork should be reviewed for necessity and downstream impact.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="go.mod">
<violation number="1" location="go.mod:7">
P2: The `byteness/keyring` fork pulls in ~17 additional indirect dependencies (Wasm runtime, 1Password SDKs, OpenTelemetry, Jaeger tracing, protobuf, etc.) compared to the original `99designs/keyring`. This significantly increases binary size and supply-chain surface for a CLI tool. Consider whether a lighter-weight fork or a pinned feature set of this fork (without optional backends) is available.</violation>
</file>
Architecture diagram
sequenceDiagram
participant User as User / CLI
participant App as ipatool (cmd)
participant KC as pkg/keychain
participant KR as CHANGED: byteness/keyring (Fork)
participant OS as OS Secure Store (Keychain/WinCred/SecretService)
Note over App,OS: Credential Management Flow
User->>App: Executes command (e.g. auth login)
App->>KC: Set(key, data)
KC->>KR: CHANGED: Open(serviceName)
KR-->>KC: keyring instance
KC->>KR: CHANGED: Set(item)
Note right of KR: Fork restores compatibility<br/>for iOS and Go 1.25
KR->>OS: NEW: Platform-specific syscalls<br/>(via byteness/go-keychain or wincred)
alt Success
OS-->>KR: OK
KR-->>KC: Success
KC-->>App: Success
else Access Denied / Platform Error
OS-->>KR: Error
KR-->>KC: Wrapped Error
KC-->>App: Return Error
end
Note over App,OS: Data Retrieval Flow
App->>KC: Get(key)
KC->>KR: CHANGED: Get(key)
KR->>OS: Request Item
OS-->>KR: Encrypted Data
KR-->>KC: Decrypted bytes
KC-->>App: Data
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
Owner
Author
|
@cubic-dev-ai Your task is to investigate the failed GitHub workflows, fix the issues that caused them to fail, then update the pull request with a new commit that contains those fixes. |
|
The CI failures were caused by outdated GitHub Actions that use deprecated Node.js runtimes:
I've updated all 5 workflow files:
✅ All tests passing (build, vet, 124 unit tests across 7 packages) Pushed commits to |
Update actions/checkout from v2 to v4 and actions/setup-go from v3 to v5 across all workflow files. The old versions use deprecated Node 12/16 runtimes that are no longer supported by GitHub Actions runners, causing CI failures.
Two issues with the CI:
1. golangci-lint v2.1 was built with Go 1.24 and cannot lint a project
targeting Go 1.25.0. Update to v2.9, which is the first release built
with Go 1.25.
2. byteness/keyring imports 1password/onepassword-sdk-go, which intentionally
fails compilation when CGO is disabled on linux/darwin targets
(client_builder_no_cgo.go, //go:build !cgo && (darwin || linux)).
Cross-compiling for Linux from macOS disables CGO by default, triggering
this error. Fix by:
- Switching Linux dry-builds and release builds to native Linux runners
(ubuntu-latest for amd64, ubuntu-24.04-arm for arm64) where CGO works
without a cross-compiler.
- Adding CGO_ENABLED: 1 to macOS dry-builds (already set in release.yml).
- Splitting Linux release builds into a dedicated build_linux job.
Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
Switch cmd/root.go to use the standard library context package. Define a typed context key (contextKey) to satisfy SA1029, which flags plain string literals as context.WithValue keys. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Switches to the maintained
github.com/byteness/keyringfork and upgrades to Go 1.25 to restore iOS builds. CI now uses Node 20 actions, native Linux runners, and CGO settings to fix build and release failures.Dependencies
github.com/99designs/keyringwithgithub.com/byteness/keyring; update imports and refreshgolang.org/x/*; add maintained platform keychain and 1Password libs (e.g.,github.com/byteness/go-keychain,github.com/byteness/go-libsecret,github.com/danieljoos/wincred).go 1.25.0ingo.mod; update workflows toactions/checkout@v4,actions/setup-go@v5, and bumpgolangci/golangci-lint-action@v8toversion: v2.9.byteness/keyring: run Linux builds on native runners (ubuntu-24.04-arm,ubuntu-latest), enableCGO_ENABLED=1for macOS dry builds, and split Linux release builds into a dedicated job.Refactors
golang.org/x/net/contextwithcontextand use a typed context key (interactiveKey) to satisfy lint rules.Written for commit 4ac3168. Summary will update on new commits.