feat(sign): guided release signing - #11
Conversation
Add a Sign phase and the anvil sign command plus anvil build --sign. internal/sign generates a PKCS12 keystore with keytool, writes key.properties, wires Gradle signingConfigs, writes an iOS ExportOptions.plist, and gitignores the secrets. iOS sign steps live on the Flutter (flutter build ipa), React Native, and native iOS (xcodebuild archive + exportArchive) drivers; Android signs at build time via the wired Gradle config. Passwords come from prompts (huh) or environment, never the repo, and --dry-run makes no changes. Tests cover live keystore generation (keytool), Gradle wiring idempotence, gitignore, ExportOptions, and step argv.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/driver/flutter.go (1)
1-1: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGate the iOS signing step on the requested target
internal/driver/flutter.goandinternal/driver/reactnative.goboth emit iOS archive/export steps inSignwheneveropts.Signing.ExportPlistis set, even when the requested target is not iOS. TheBuildcase already checks the target, soSignshould use the same guard before generating iOS signing steps.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/driver/flutter.go` at line 1, Update the Sign implementations in the Flutter and React Native drivers to generate iOS archive/export signing steps only when the requested target is iOS and opts.Signing.ExportPlist is set. Reuse the target guard already applied by their Build paths, leaving signing behavior unchanged for iOS requests.internal/driver/ios.go (1)
1-1: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClear the export directory before
xcodebuild -exportArchiveBoth iOS sign steps write to the fixed
build/anvil/ipapath. Because that directory is never removed or uniquified, a repeatanvil sign/anvil build --signrun on the same checkout can fail once the export path already exists. Clear it first or use a per-run unique export path in bothinternal/driver/ios.goandinternal/driver/reactnative.go.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/driver/ios.go` at line 1, Update the iOS signing flows in the relevant methods of ios.go and reactnative.go to remove or uniquify the fixed build/anvil/ipa export directory before invoking xcodebuild -exportArchive. Apply the same behavior to both sign paths while preserving the existing archive export configuration.
🧹 Nitpick comments (4)
internal/sign/sign.go (1)
40-40: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse
exec.CommandContextfor thekeytoolinvocation.golangci-lint's
noctxflags this: there's no way to bound/cancel a hangingkeytoolcall.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/sign/sign.go` at line 40, Update the keytool invocation in the CombinedOutput error-handling flow to use exec.CommandContext with the appropriate existing context, preserving the current arguments and output handling while allowing the process to be cancelled or bounded.Source: Linters/SAST tools
internal/driver/driver_test.go (1)
124-151: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo test coverage for ReactNative's Sign phase, and no test asserts Sign behavior when
Targetconflicts withSigning.
TestFlutterSignSteps/TestIOSSignStepscover the happy path, but there's noTestReactNativeSignSteps, and none of these tests setTargetalongsideSigningto catch the target-gating gap flagged in flutter.go/reactnative.go.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/driver/driver_test.go` around lines 124 - 151, The driver tests lack Sign-phase coverage for ReactNative and do not verify behavior when Target conflicts with Signing. Add a TestReactNativeSignSteps covering the configured signing command, and update the Flutter and iOS Sign tests to provide a conflicting Target alongside Signing, asserting signing remains correctly gated and produces the expected steps.internal/driver/reactnative.go (1)
58-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated archive/export construction vs. ios.go; also reuses a fixed export path across runs.
This closely mirrors
IOS.Steps'sSigncase (archive → export, samebuild/anvil/...paths, same DEVELOPMENT_TEAM handling). Beyond the duplication,xcodebuild -exportArchivefails when-exportPathalready exists, so re-running Sign on this tree without clearingbuild/anvil/ipawill fail — same as ios.go.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/driver/reactnative.go` around lines 58 - 76, Update the signing flow in the relevant React Native step construction to reuse the existing iOS archive/export implementation, such as IOS.Steps, instead of duplicating archiveArgs and export-step assembly. Ensure the shared implementation uses a fresh or cleaned export path for each run so repeated Sign executions do not fail when build/anvil/ipa already exists, while preserving workspace, scheme, configuration, archive, and DEVELOPMENT_TEAM behavior.cmd/build.go (1)
62-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
anvil build --signcan't target iOS for dual-platform (Flutter/RN) projects.
setupSigning(cmd, chosen, "", buildDryRun)always passes an empty platform, andresolvePlatformmaps Flutter/ReactNative/Android to"android"unconditionally (cmd/sign.go Line 94-95). Unlikeanvil sign, which exposes--platformfor explicit override,anvil build --signhas no equivalent flag, so a Flutter project can never get a signed iOS build throughbuild --sign— onlyandroid.Consider adding a
--platformflag tobuildCmdand threading it through tosetupSigningthe same waysignPlatformis used incmd/sign.go.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/build.go` around lines 62 - 69, Update buildCmd to expose a --platform option and propagate its value through the build signing flow: replace the empty platform argument in setupSigning with the build command’s platform value, matching the signPlatform handling in the sign command. Preserve existing behavior when no platform is specified.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/sign.go`:
- Around line 164-173: Validate the prompted teamID after the interactive huh
form completes and reject an empty value before continuing to signing. Match the
existing empty-value handling in resolveSecret, returning an appropriate error
instead of allowing sign.WriteExportOptions to receive a blank teamID.
- Around line 63-72: Update the branching in runSign so signDryRun is handled
before the len(extra) == 0 success message, allowing Android dry runs to call
printPlan and return without claiming signing setup completed. Preserve the
existing configured message for non-dry-run flows with no extra signing phase.
In `@internal/driver/ios.go`:
- Around line 41-60: The iOS signing export path in the Sign case is reused
across runs and must be cleaned or made unique before export. Update the
archive/export flow around the archive variable and the “xcodebuild
-exportArchive” Step so each invocation uses an empty or uniquely identified
export directory, while keeping the archive and exportOptionsPlist behavior
unchanged.
In `@internal/driver/reactnative.go`:
- Around line 54-76: The Sign branch in the React Native step generation must
only emit iOS archive/export steps for an iOS target. Update the guard before
the workspace lookup to require the appropriate target alongside
Signing.ExportPlist, while preserving the existing workspace and signing
configuration behavior.
In `@internal/sign/sign.go`:
- Around line 112-121: Handle and propagate errors from deferred file closes in
both WireAndroidGradle and EnsureGitignore. Update each function’s cleanup so a
failed f.Close() returns false with the close error, while preserving successful
write and return behavior.
- Around line 32-42: Update the keytool invocation in the genkeypair flow to
stop placing ks.StorePass and ks.KeyPass directly in argv. Pass both passwords
through keytool’s supported :env or :file modifiers, configuring the required
environment variables or temporary password files while preserving existing
error handling and cleanup.
---
Outside diff comments:
In `@internal/driver/flutter.go`:
- Line 1: Update the Sign implementations in the Flutter and React Native
drivers to generate iOS archive/export signing steps only when the requested
target is iOS and opts.Signing.ExportPlist is set. Reuse the target guard
already applied by their Build paths, leaving signing behavior unchanged for iOS
requests.
In `@internal/driver/ios.go`:
- Line 1: Update the iOS signing flows in the relevant methods of ios.go and
reactnative.go to remove or uniquify the fixed build/anvil/ipa export directory
before invoking xcodebuild -exportArchive. Apply the same behavior to both sign
paths while preserving the existing archive export configuration.
---
Nitpick comments:
In `@cmd/build.go`:
- Around line 62-69: Update buildCmd to expose a --platform option and propagate
its value through the build signing flow: replace the empty platform argument in
setupSigning with the build command’s platform value, matching the signPlatform
handling in the sign command. Preserve existing behavior when no platform is
specified.
In `@internal/driver/driver_test.go`:
- Around line 124-151: The driver tests lack Sign-phase coverage for ReactNative
and do not verify behavior when Target conflicts with Signing. Add a
TestReactNativeSignSteps covering the configured signing command, and update the
Flutter and iOS Sign tests to provide a conflicting Target alongside Signing,
asserting signing remains correctly gated and produces the expected steps.
In `@internal/driver/reactnative.go`:
- Around line 58-76: Update the signing flow in the relevant React Native step
construction to reuse the existing iOS archive/export implementation, such as
IOS.Steps, instead of duplicating archiveArgs and export-step assembly. Ensure
the shared implementation uses a fresh or cleaned export path for each run so
repeated Sign executions do not fail when build/anvil/ipa already exists, while
preserving workspace, scheme, configuration, archive, and DEVELOPMENT_TEAM
behavior.
In `@internal/sign/sign.go`:
- Line 40: Update the keytool invocation in the CombinedOutput error-handling
flow to use exec.CommandContext with the appropriate existing context,
preserving the current arguments and output handling while allowing the process
to be cancelled or bounded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 44d03644-c0c1-4053-931f-efb2699a1d84
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (15)
CHANGELOG.mdcmd/build.gocmd/sign.godocs/ROADMAP.mdgo.modinternal/driver/android.gointernal/driver/driver.gointernal/driver/driver_test.gointernal/driver/flutter.gointernal/driver/ios.gointernal/driver/reactnative.gointernal/pipeline/pipeline.gointernal/sign/sign.gointernal/sign/sign_test.gotasks/todo.md
💤 Files with no reviewable changes (1)
- internal/driver/android.go
| if len(extra) == 0 { | ||
| fmt.Fprintln(cmd.OutOrStdout(), "Signing configured. Run 'anvil build --release' to produce a signed artifact.") | ||
| return nil | ||
| } | ||
| if signDryRun { | ||
| printPlan(cmd, chosen, d, opts, extra) | ||
| return nil | ||
| } | ||
| return runPipeline(cmd, chosen.Path, d, opts, extra) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
--dry-run prints a false "Signing configured" success message for Android.
setupAndroidSigning's dry-run branch always returns nil for extra (Android never needs an extra Sign phase), so back in runSign, len(extra) == 0 is true and the "Signing configured. Run 'anvil build --release'..." message fires unconditionally — even though signDryRun is true and nothing was actually written. A user running anvil sign --dry-run on an Android project sees "Would set up Android signing:" immediately followed by a message implying setup is complete.
🐛 Proposed fix
- if len(extra) == 0 {
- fmt.Fprintln(cmd.OutOrStdout(), "Signing configured. Run 'anvil build --release' to produce a signed artifact.")
- return nil
- }
- if signDryRun {
- printPlan(cmd, chosen, d, opts, extra)
- return nil
- }
+ if signDryRun {
+ if len(extra) > 0 {
+ printPlan(cmd, chosen, d, opts, extra)
+ }
+ return nil
+ }
+ if len(extra) == 0 {
+ fmt.Fprintln(cmd.OutOrStdout(), "Signing configured. Run 'anvil build --release' to produce a signed artifact.")
+ return nil
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if len(extra) == 0 { | |
| fmt.Fprintln(cmd.OutOrStdout(), "Signing configured. Run 'anvil build --release' to produce a signed artifact.") | |
| return nil | |
| } | |
| if signDryRun { | |
| printPlan(cmd, chosen, d, opts, extra) | |
| return nil | |
| } | |
| return runPipeline(cmd, chosen.Path, d, opts, extra) | |
| } | |
| if signDryRun { | |
| if len(extra) > 0 { | |
| printPlan(cmd, chosen, d, opts, extra) | |
| } | |
| return nil | |
| } | |
| if len(extra) == 0 { | |
| fmt.Fprintln(cmd.OutOrStdout(), "Signing configured. Run 'anvil build --release' to produce a signed artifact.") | |
| return nil | |
| } | |
| return runPipeline(cmd, chosen.Path, d, opts, extra) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/sign.go` around lines 63 - 72, Update the branching in runSign so
signDryRun is handled before the len(extra) == 0 success message, allowing
Android dry runs to call printPlan and return without claiming signing setup
completed. Preserve the existing configured message for non-dry-run flows with
no extra signing phase.
| if teamID == "" { | ||
| if !interactive() { | ||
| return driver.Signing{}, nil, errors.New("set --team-id or ANVIL_TEAM_ID") | ||
| } | ||
| if err := huh.NewForm(huh.NewGroup( | ||
| huh.NewInput().Title("Apple Developer Team ID").Value(&teamID), | ||
| )).Run(); err != nil { | ||
| return driver.Signing{}, nil, err | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Missing empty-value validation for the interactive Team ID prompt.
Unlike resolveSecret (Line 196-198), which errors out if the prompted value is empty, this interactive Team ID prompt has no such check. A blank submission proceeds to sign.WriteExportOptions with an empty teamID, silently producing an incomplete ExportOptions.plist.
🐛 Proposed fix
if err := huh.NewForm(huh.NewGroup(
huh.NewInput().Title("Apple Developer Team ID").Value(&teamID),
)).Run(); err != nil {
return driver.Signing{}, nil, err
}
+ if teamID == "" {
+ return driver.Signing{}, nil, errors.New("Apple Developer Team ID is required")
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if teamID == "" { | |
| if !interactive() { | |
| return driver.Signing{}, nil, errors.New("set --team-id or ANVIL_TEAM_ID") | |
| } | |
| if err := huh.NewForm(huh.NewGroup( | |
| huh.NewInput().Title("Apple Developer Team ID").Value(&teamID), | |
| )).Run(); err != nil { | |
| return driver.Signing{}, nil, err | |
| } | |
| } | |
| if teamID == "" { | |
| if !interactive() { | |
| return driver.Signing{}, nil, errors.New("set --team-id or ANVIL_TEAM_ID") | |
| } | |
| if err := huh.NewForm(huh.NewGroup( | |
| huh.NewInput().Title("Apple Developer Team ID").Value(&teamID), | |
| )).Run(); err != nil { | |
| return driver.Signing{}, nil, err | |
| } | |
| if teamID == "" { | |
| return driver.Signing{}, nil, errors.New("Apple Developer Team ID is required") | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/sign.go` around lines 164 - 173, Validate the prompted teamID after the
interactive huh form completes and reject an empty value before continuing to
signing. Match the existing empty-value handling in resolveSecret, returning an
appropriate error instead of allowing sign.WriteExportOptions to receive a blank
teamID.
| case Sign: | ||
| flag, value, scheme := d.container(opts) | ||
| if scheme == "" { | ||
| return nil, false | ||
| } | ||
| archive := "build/anvil/" + scheme + ".xcarchive" | ||
| plist := opts.Signing.ExportPlist | ||
| if plist == "" { | ||
| plist = "ExportOptions.plist" | ||
| } | ||
| archiveArgs := []string{"xcodebuild", flag, value, "-scheme", scheme, | ||
| "-configuration", "Release", "-archivePath", archive, "archive", "-allowProvisioningUpdates"} | ||
| if opts.Signing.TeamID != "" { | ||
| archiveArgs = append(archiveArgs, "DEVELOPMENT_TEAM="+opts.Signing.TeamID) | ||
| } | ||
| return []Step{ | ||
| {Name: "xcodebuild archive", Argv: archiveArgs}, | ||
| {Name: "xcodebuild -exportArchive", Argv: []string{"xcodebuild", "-exportArchive", | ||
| "-archivePath", archive, "-exportPath", "build/anvil/ipa", "-exportOptionsPlist", plist}}, | ||
| }, true |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,120p' internal/driver/ios.goRepository: openforge-oss/anvil
Length of output: 2590
🏁 Script executed:
rg -n "exportPath|exportArchive|ExportOptions.plist|build/anvil/ipa" internal -SRepository: openforge-oss/anvil
Length of output: 1594
🌐 Web query:
xcodebuild -exportArchive -exportPath already exists behavior documentation
💡 Result:
The behavior of xcodebuild when a target file or path already exists depends on the specific flag or context: 1. Result Bundles (-resultBundlePath): If you use the -resultBundlePath flag to specify a path for test results, xcodebuild will fail and exit with an error if that path already exists [1][2]. It does not automatically overwrite existing bundles [2]. 2. General Build Products: In standard build operations, xcodebuild manages the build directory (typically defined by SYMROOT or DERIVED_DATA_DIR) based on incremental build logic. If files already exist in these directories, xcodebuild uses them to optimize build times [3][4]. If it detects multiple build commands producing the same output file, it will issue a warning (e.g., "Multiple build commands for output file") [5]. 3. Clean Builds: If you wish to ensure a fresh environment where existing build artifacts are removed, you must explicitly use the clean action (e.g., xcodebuild clean) before your build command [6][7]. Note that the clean action removes build products and intermediate files but may not delete the entire directory structure in the same way as manual deletion [7]. In summary, xcodebuild does not indiscriminately overwrite all existing files; it is designed for incremental builds and will error out if specific output paths (like result bundles) are pre-occupied [1][2][3].
Citations:
- 1: https://leancrew.com/all-this/man/man1/xcodebuild.html
- 2: scan with result_bundle cannot be run twice: xcodebuild resultBundlePath existing file fastlane/fastlane#12349
- 3: https://medium.com/swiftblade/what-the-hell-is-this-output-files-in-xcode-build-phases-bfbec6391184
- 4: https://developer.apple.com/library/archive/technotes/tn2339/_index.html
- 5: https://stackoverflow.com/questions/2718246/xcode-warning-multiple-build-commands-for-output-file
- 6: https://www.manpagez.com/man/1/xcodebuild/osx-10.5.php
- 7: https://stackoverflow.com/questions/49434338/can-xcodebuild-delete-the-contents-of-the-projects-build-folder
🌐 Web query:
"xcodebuild -exportArchive -exportPath already exists" "ExportPath"
💡 Result:
When using the xcodebuild command with the -exportArchive option, the -exportPath flag specifies the destination directory for the exported product [1][2]. If you encounter an error indicating that a file or directory at the -exportPath already exists, it is typically because xcodebuild attempts to write the exported artifacts (such as the.ipa file, DistributionSummary.plist, and logging bundles) into that location [3][1]. xcodebuild does not natively support an "overwrite" flag to automatically clear or force-replace existing contents at that path. To resolve this issue, you must ensure the -exportPath directory is clean before running the command. Common strategies include: 1. Manual Cleanup: Add a command to your script (e.g., rm -rf "$EXPORT_PATH") to delete the contents of the directory or the directory itself before executing the xcodebuild command. 2. Unique Pathing: Generate a unique export path for every build, such as by appending a timestamp or a unique build identifier to the directory name [3]. This is a best practice in CI/CD environments to avoid collisions between parallel or sequential builds. 3. Temporary Directories: If you do not need to persist the exported files, use a temporary directory (e.g., /tmp/build_output) and clean it up after the process completes. Note that attempting to provide the -exportPath argument more than once in a single command will result in an error, as the option may only be specified once [4]. Additionally, some errors related to pre-existing files (such as.xcdistributionlogs) can occur in temporary system directories that are outside of your explicit -exportPath, which may require clearing system temp folders if they become corrupted or locked [3][5].
Citations:
- 1: https://leancrew.com/all-this/man/man1/xcodebuild.html
- 2: https://gist.github.com/palaniraja/4e58f40cae21105d532952a5c6eb1b30
- 3: Running multiple ios builds in parallel using gym fails fastlane/fastlane#21514
- 4: Can't change exportPath fastlane/fastlane#12739
- 5: [Question]: Xcode export fail with no meaningful error message microsoft/azure-pipelines-tasks#20104
Clean or uniquify the export path for iOS signing. build/anvil/ipa is reused on every run, and xcodebuild -exportArchive won't overwrite an existing export directory. A second anvil sign / anvil build --sign on the same tree will fail unless that path is cleared first.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/driver/ios.go` around lines 41 - 60, The iOS signing export path in
the Sign case is reused across runs and must be cleaned or made unique before
export. Update the archive/export flow around the archive variable and the
“xcodebuild -exportArchive” Step so each invocation uses an empty or uniquely
identified export directory, while keeping the archive and exportOptionsPlist
behavior unchanged.
| case Sign: | ||
| if opts.Signing.ExportPlist == "" { | ||
| return nil, false | ||
| } | ||
| ws := firstGlob(filepath.Join(r.root, "ios"), "*.xcworkspace") | ||
| if ws == "" { | ||
| return nil, false | ||
| } | ||
| scheme := strings.TrimSuffix(ws, ".xcworkspace") | ||
| if opts.Flavor != "" { | ||
| scheme = opts.Flavor | ||
| } | ||
| archive := "build/anvil/" + scheme + ".xcarchive" | ||
| archiveArgs := []string{"xcodebuild", "-workspace", ws, "-scheme", scheme, | ||
| "-configuration", "Release", "-archivePath", archive, "archive", "-allowProvisioningUpdates"} | ||
| if opts.Signing.TeamID != "" { | ||
| archiveArgs = append(archiveArgs, "DEVELOPMENT_TEAM="+opts.Signing.TeamID) | ||
| } | ||
| return []Step{ | ||
| {Name: "xcodebuild archive", Dir: "ios", Argv: archiveArgs}, | ||
| {Name: "xcodebuild -exportArchive", Dir: "ios", Argv: []string{"xcodebuild", "-exportArchive", | ||
| "-archivePath", archive, "-exportPath", "build/anvil/ipa", "-exportOptionsPlist", opts.Signing.ExportPlist}}, | ||
| }, true |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== reactnative.go outline ==\n'
ast-grep outline internal/driver/reactnative.go --view expanded || true
printf '\n== ios.go outline ==\n'
ast-grep outline internal/driver/ios.go --view expanded || true
printf '\n== flutter.go outline ==\n'
ast-grep outline internal/driver/flutter.go --view expanded || true
printf '\n== target helper search ==\n'
rg -n "func target|target\\(" internal/driver -g '*.go'
printf '\n== reactnative.go relevant lines ==\n'
cat -n internal/driver/reactnative.go | sed -n '1,140p'
printf '\n== ios.go relevant lines ==\n'
cat -n internal/driver/ios.go | sed -n '1,160p'
printf '\n== flutter.go relevant lines ==\n'
cat -n internal/driver/flutter.go | sed -n '1,180p'Repository: openforge-oss/anvil
Length of output: 11385
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== search for ExportPlist and Sign cases ==\n'
rg -n "ExportPlist|case Sign|xcodebuild -exportArchive|target\\(opts, \"android\"\\)" internal -g '*.go'
printf '\n== top of internal/driver/reactnative.go ==\n'
cat -n internal/driver/reactnative.go | sed -n '1,220p'Repository: openforge-oss/anvil
Length of output: 6001
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== driver.go relevant lines ==\n'
cat -n internal/driver/driver.go | sed -n '1,220p'
printf '\n== driver tests around sign ==\n'
cat -n internal/driver/driver_test.go | sed -n '100,180p'
printf '\n== search reactnative tests ==\n'
rg -n "react-native|ReactNative|Signing|ExportPlist|Target" internal/driver -g '*test.go'Repository: openforge-oss/anvil
Length of output: 8072
Guard React Native sign steps by target. Sign only checks Signing.ExportPlist, so an Android-targeted run can still emit the iOS archive/export steps.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/driver/reactnative.go` around lines 54 - 76, The Sign branch in the
React Native step generation must only emit iOS archive/export steps for an iOS
target. Update the guard before the workspace lookup to require the appropriate
target alongside Signing.ExportPlist, while preserving the existing workspace
and signing configuration behavior.
| args := []string{ | ||
| "-genkeypair", "-noprompt", | ||
| "-keystore", ks.Path, "-alias", ks.Alias, | ||
| "-keyalg", "RSA", "-keysize", "2048", "-validity", "10000", | ||
| "-storetype", "PKCS12", | ||
| "-storepass", ks.StorePass, "-keypass", ks.KeyPass, | ||
| "-dname", ks.DName, | ||
| } | ||
| if out, err := exec.Command("keytool", args...).CombinedOutput(); err != nil { | ||
| return fmt.Errorf("keytool genkeypair: %w: %s", err, strings.TrimSpace(string(out))) | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,120p' internal/sign/sign.go | cat -nRepository: openforge-oss/anvil
Length of output: 4992
🌐 Web query:
Oracle keytool -storepass:env -keypass:env documentation password should not be specified on a command line or in a script
💡 Result:
Oracle keytool documentation explicitly advises against specifying passwords directly on a command line or within a script to maintain security, stating that passwords should only be handled this way for testing purposes or on systems specifically secured for such actions [1][2][3]. When a password is required but not provided on the command line, keytool will prompt the user to enter it securely [1][2]. To manage passwords securely without hardcoding them in scripts, Oracle's keytool documentation specifies that certain password-related options (such as -storepass, -keypass, -srckeypass, -destkeypass, -srcstorepass, and -deststorepass) support modifiers to retrieve the password from a secure source [4][5]: - Environment Variables: You can use the:env modifier followed by the name of an environment variable (e.g., -storepass:env:MY_PASSWORD_VAR) [4][5]. - Files: You can use the:file modifier followed by the path to a file containing the password (e.g., -storepass:file:/path/to/password.txt) [4][5]. For additional security, best practices include setting restrictive file permissions (e.g., chmod 600) on any password files and using secret management tools to handle sensitive credentials [6][7]. Avoid committing scripts containing passwords to version control [8].
Citations:
- 1: https://docs.oracle.com/en/java/javase/26/docs/specs/man/keytool.html
- 2: https://docs.oracle.com/javase/8/docs/technotes/tools/windows/keytool.html
- 3: https://dev.java/learn/jvm/tool/security/keytool/
- 4: https://docs.oracle.com/javase/8/docs/technotes/tools/unix/keytool.html
- 5: https://github.com/openjdk/jdk/blob/master/src/java.base/share/man/keytool.md
- 6: https://myarch.com/cert-book/keystore_best_practices.html
- 7: https://fixmycert.com/guides/jks-fundamentals
- 8: https://www.mojohaus.org/keytool/faq.html
Avoid passing keystore passwords on the command line
-storepass/-keypass expose secrets in argv (ps//proc/<pid>/cmdline). Use keytool’s :env or :file password modifiers instead.
🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 40-40: os/exec.Command must not be called. use os/exec.CommandContext
(noctx)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/sign/sign.go` around lines 32 - 42, Update the keytool invocation in
the genkeypair flow to stop placing ks.StorePass and ks.KeyPass directly in
argv. Pass both passwords through keytool’s supported :env or :file modifiers,
configuring the required environment variables or temporary password files while
preserving existing error handling and cleanup.
| f, err := os.OpenFile(appBuildFile, os.O_APPEND|os.O_WRONLY, 0o644) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
| defer f.Close() | ||
| if _, err := f.WriteString(block); err != nil { | ||
| return false, err | ||
| } | ||
| return true, nil | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Unchecked f.Close() errors.
Both WireAndroidGradle and EnsureGitignore defer-close the file without checking the error, flagged by errcheck. A failed close on write could silently drop signing-config/gitignore content.
🩹 Proposed fix pattern
- defer f.Close()
if _, err := f.WriteString(block); err != nil {
return false, err
}
- return true, nil
+ if err := f.Close(); err != nil {
+ return false, err
+ }
+ return true, nilAlso applies to: 164-171
🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 116-116: Error return value of f.Close is not checked
(errcheck)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/sign/sign.go` around lines 112 - 121, Handle and propagate errors
from deferred file closes in both WireAndroidGradle and EnsureGitignore. Update
each function’s cleanup so a failed f.Close() returns false with the close
error, while preserving successful write and return behavior.
Source: Linters/SAST tools
* feat(detect): stack detection engine and anvil detect command Add internal/detect with marker-file detectors for Flutter (vs pure Dart; app/module/plugin), React Native (bare, Expo managed, Expo prebuild), native Android (app vs library, KMP flag), and native iOS (Xcode/SPM/Podfile). A depth-limited prune-on-detect scanner with skip lists and a containment sweep attributes android/ios folders to their Flutter or RN parent and surfaces each monorepo project once. Add the Cobra-based anvil detect command (table and --json, with --path and --depth). 16 fixture-tree tests cover the edge cases (node_modules exclusion, monorepo, KMP, plugin example pruning). Also apply the no-emojis, no-dash-connectors, minimal-comments style across docs and record it in CLAUDE.md. * feat(build): guided build lifecycle and anvil build (#3) * feat(build): guided build lifecycle and anvil build Add the driver contract (internal/driver) and drivers for Flutter, React Native, native Android, native iOS, Swift (SPM), and Kotlin/JVM, with --flavor threaded into build and test steps. Add the runner (internal/pipeline) that streams combined output, captures exit codes, classifies results, collects artifacts, and fail-fasts. Add internal/tui with a Bubble Tea view and a plain non-TTY renderer, and the anvil build command (--path/--target/--flavor/--release/--dry-run/--plain). Refine detection so Gradle is not always Android and Package.swift is Swift, adding Swift and Kotlin stacks. Tests cover driver steps, the runner via a real subprocess, and the plain renderer. * fix: remove unused dirExists helper (staticcheck U1000) * feat(sign): guided release signing (#11) Add a Sign phase and the anvil sign command plus anvil build --sign. internal/sign generates a PKCS12 keystore with keytool, writes key.properties, wires Gradle signingConfigs, writes an iOS ExportOptions.plist, and gitignores the secrets. iOS sign steps live on the Flutter (flutter build ipa), React Native, and native iOS (xcodebuild archive + exportArchive) drivers; Android signs at build time via the wired Gradle config. Passwords come from prompts (huh) or environment, never the repo, and --dry-run makes no changes. Tests cover live keystore generation (keytool), Gradle wiring idempotence, gitignore, ExportOptions, and step argv.
* feat(detect): stack detection engine and anvil detect command Add internal/detect with marker-file detectors for Flutter (vs pure Dart; app/module/plugin), React Native (bare, Expo managed, Expo prebuild), native Android (app vs library, KMP flag), and native iOS (Xcode/SPM/Podfile). A depth-limited prune-on-detect scanner with skip lists and a containment sweep attributes android/ios folders to their Flutter or RN parent and surfaces each monorepo project once. Add the Cobra-based anvil detect command (table and --json, with --path and --depth). 16 fixture-tree tests cover the edge cases (node_modules exclusion, monorepo, KMP, plugin example pruning). Also apply the no-emojis, no-dash-connectors, minimal-comments style across docs and record it in CLAUDE.md. * feat(build): guided build lifecycle and anvil build (#3) * feat(build): guided build lifecycle and anvil build Add the driver contract (internal/driver) and drivers for Flutter, React Native, native Android, native iOS, Swift (SPM), and Kotlin/JVM, with --flavor threaded into build and test steps. Add the runner (internal/pipeline) that streams combined output, captures exit codes, classifies results, collects artifacts, and fail-fasts. Add internal/tui with a Bubble Tea view and a plain non-TTY renderer, and the anvil build command (--path/--target/--flavor/--release/--dry-run/--plain). Refine detection so Gradle is not always Android and Package.swift is Swift, adding Swift and Kotlin stacks. Tests cover driver steps, the runner via a real subprocess, and the plain renderer. * fix: remove unused dirExists helper (staticcheck U1000) * feat(sign): guided release signing (#11) Add a Sign phase and the anvil sign command plus anvil build --sign. internal/sign generates a PKCS12 keystore with keytool, writes key.properties, wires Gradle signingConfigs, writes an iOS ExportOptions.plist, and gitignores the secrets. iOS sign steps live on the Flutter (flutter build ipa), React Native, and native iOS (xcodebuild archive + exportArchive) drivers; Android signs at build time via the wired Gradle config. Passwords come from prompts (huh) or environment, never the repo, and --dry-run makes no changes. Tests cover live keystore generation (keytool), Gradle wiring idempotence, gitignore, ExportOptions, and step argv. * feat(upload): store upload and GoReleaser self-distribution (#13) Add internal/upload with an Uploader interface and three targets: iOS via xcrun altool (App Store Connect/TestFlight), Android via the Google Play Publisher API (insert edit, upload bundle, assign track, commit) using a service-account JWT, and npm via npm publish. Credentials resolve from flag, env, or a base64 env decoded to a 0600 temp file, and are refused if they live inside the repo. anvil upload is a dry run unless --yes. Add GoReleaser (.goreleaser.yaml) and a tag-triggered release workflow that build cross-platform binaries and publish a GitHub release plus Homebrew cask and Scoop manifests. Tests cover credential resolution, the in-repo refusal, and each uploader's Validate/Describe. * feat(detect,driver): Go and web/Node stacks (#14) Add Go and web/Node as detectors and drivers, reusing the existing contract with no new dependencies. Go: go.mod detection (app vs library via a main-package scan); go mod download, go vet + gofmt -l (Analyze classified as failed on non-empty output), go test, go build. Web/Node: framework detection (Next, Nuxt, SvelteKit, Angular, Vite, CRA, Vue, Svelte, Astro, Remix, Gatsby); deps by lockfile (with Yarn Berry --immutable), lint, test (jest/vitest/script with CI=true), build via the package.json script. Monorepo roots (workspaces, pnpm-workspace.yaml, lerna, nx) are descended into rather than claimed, and a single-package Turbo repo is a leaf. Detector order appends Go then Web (Web last, most permissive).
* feat(detect): stack detection engine and anvil detect command Add internal/detect with marker-file detectors for Flutter (vs pure Dart; app/module/plugin), React Native (bare, Expo managed, Expo prebuild), native Android (app vs library, KMP flag), and native iOS (Xcode/SPM/Podfile). A depth-limited prune-on-detect scanner with skip lists and a containment sweep attributes android/ios folders to their Flutter or RN parent and surfaces each monorepo project once. Add the Cobra-based anvil detect command (table and --json, with --path and --depth). 16 fixture-tree tests cover the edge cases (node_modules exclusion, monorepo, KMP, plugin example pruning). Also apply the no-emojis, no-dash-connectors, minimal-comments style across docs and record it in CLAUDE.md. * feat(build): guided build lifecycle and anvil build (#3) * feat(build): guided build lifecycle and anvil build Add the driver contract (internal/driver) and drivers for Flutter, React Native, native Android, native iOS, Swift (SPM), and Kotlin/JVM, with --flavor threaded into build and test steps. Add the runner (internal/pipeline) that streams combined output, captures exit codes, classifies results, collects artifacts, and fail-fasts. Add internal/tui with a Bubble Tea view and a plain non-TTY renderer, and the anvil build command (--path/--target/--flavor/--release/--dry-run/--plain). Refine detection so Gradle is not always Android and Package.swift is Swift, adding Swift and Kotlin stacks. Tests cover driver steps, the runner via a real subprocess, and the plain renderer. * fix: remove unused dirExists helper (staticcheck U1000) * feat(sign): guided release signing (#11) Add a Sign phase and the anvil sign command plus anvil build --sign. internal/sign generates a PKCS12 keystore with keytool, writes key.properties, wires Gradle signingConfigs, writes an iOS ExportOptions.plist, and gitignores the secrets. iOS sign steps live on the Flutter (flutter build ipa), React Native, and native iOS (xcodebuild archive + exportArchive) drivers; Android signs at build time via the wired Gradle config. Passwords come from prompts (huh) or environment, never the repo, and --dry-run makes no changes. Tests cover live keystore generation (keytool), Gradle wiring idempotence, gitignore, ExportOptions, and step argv. * feat(upload): store upload and GoReleaser self-distribution (#13) Add internal/upload with an Uploader interface and three targets: iOS via xcrun altool (App Store Connect/TestFlight), Android via the Google Play Publisher API (insert edit, upload bundle, assign track, commit) using a service-account JWT, and npm via npm publish. Credentials resolve from flag, env, or a base64 env decoded to a 0600 temp file, and are refused if they live inside the repo. anvil upload is a dry run unless --yes. Add GoReleaser (.goreleaser.yaml) and a tag-triggered release workflow that build cross-platform binaries and publish a GitHub release plus Homebrew cask and Scoop manifests. Tests cover credential resolution, the in-repo refusal, and each uploader's Validate/Describe. * feat(detect,driver): Go and web/Node stacks (#14) Add Go and web/Node as detectors and drivers, reusing the existing contract with no new dependencies. Go: go.mod detection (app vs library via a main-package scan); go mod download, go vet + gofmt -l (Analyze classified as failed on non-empty output), go test, go build. Web/Node: framework detection (Next, Nuxt, SvelteKit, Angular, Vite, CRA, Vue, Svelte, Astro, Remix, Gatsby); deps by lockfile (with Yarn Berry --immutable), lint, test (jest/vitest/script with CI=true), build via the package.json script. Monorepo roots (workspaces, pnpm-workspace.yaml, lerna, nx) are descended into rather than claimed, and a single-package Turbo repo is a leaf. Detector order appends Go then Web (Web last, most permissive). * chore(release): prepare v0.1.0 (#16) Rewrite the README for the released tool (eight stacks, full detect/build/sign/upload pipeline, install via binary or go install, usage and flags). Make Homebrew and Scoop upload skip gracefully when HOMEBREW_TAP_TOKEN is absent, so the release ships binaries and the GitHub release without a token and auto-enables tap publishing once the secret is set.
* feat(detect): stack detection engine and anvil detect command Add internal/detect with marker-file detectors for Flutter (vs pure Dart; app/module/plugin), React Native (bare, Expo managed, Expo prebuild), native Android (app vs library, KMP flag), and native iOS (Xcode/SPM/Podfile). A depth-limited prune-on-detect scanner with skip lists and a containment sweep attributes android/ios folders to their Flutter or RN parent and surfaces each monorepo project once. Add the Cobra-based anvil detect command (table and --json, with --path and --depth). 16 fixture-tree tests cover the edge cases (node_modules exclusion, monorepo, KMP, plugin example pruning). Also apply the no-emojis, no-dash-connectors, minimal-comments style across docs and record it in CLAUDE.md. * feat(build): guided build lifecycle and anvil build (#3) * feat(build): guided build lifecycle and anvil build Add the driver contract (internal/driver) and drivers for Flutter, React Native, native Android, native iOS, Swift (SPM), and Kotlin/JVM, with --flavor threaded into build and test steps. Add the runner (internal/pipeline) that streams combined output, captures exit codes, classifies results, collects artifacts, and fail-fasts. Add internal/tui with a Bubble Tea view and a plain non-TTY renderer, and the anvil build command (--path/--target/--flavor/--release/--dry-run/--plain). Refine detection so Gradle is not always Android and Package.swift is Swift, adding Swift and Kotlin stacks. Tests cover driver steps, the runner via a real subprocess, and the plain renderer. * fix: remove unused dirExists helper (staticcheck U1000) * feat(sign): guided release signing (#11) Add a Sign phase and the anvil sign command plus anvil build --sign. internal/sign generates a PKCS12 keystore with keytool, writes key.properties, wires Gradle signingConfigs, writes an iOS ExportOptions.plist, and gitignores the secrets. iOS sign steps live on the Flutter (flutter build ipa), React Native, and native iOS (xcodebuild archive + exportArchive) drivers; Android signs at build time via the wired Gradle config. Passwords come from prompts (huh) or environment, never the repo, and --dry-run makes no changes. Tests cover live keystore generation (keytool), Gradle wiring idempotence, gitignore, ExportOptions, and step argv. * feat(upload): store upload and GoReleaser self-distribution (#13) Add internal/upload with an Uploader interface and three targets: iOS via xcrun altool (App Store Connect/TestFlight), Android via the Google Play Publisher API (insert edit, upload bundle, assign track, commit) using a service-account JWT, and npm via npm publish. Credentials resolve from flag, env, or a base64 env decoded to a 0600 temp file, and are refused if they live inside the repo. anvil upload is a dry run unless --yes. Add GoReleaser (.goreleaser.yaml) and a tag-triggered release workflow that build cross-platform binaries and publish a GitHub release plus Homebrew cask and Scoop manifests. Tests cover credential resolution, the in-repo refusal, and each uploader's Validate/Describe. * feat(detect,driver): Go and web/Node stacks (#14) Add Go and web/Node as detectors and drivers, reusing the existing contract with no new dependencies. Go: go.mod detection (app vs library via a main-package scan); go mod download, go vet + gofmt -l (Analyze classified as failed on non-empty output), go test, go build. Web/Node: framework detection (Next, Nuxt, SvelteKit, Angular, Vite, CRA, Vue, Svelte, Astro, Remix, Gatsby); deps by lockfile (with Yarn Berry --immutable), lint, test (jest/vitest/script with CI=true), build via the package.json script. Monorepo roots (workspaces, pnpm-workspace.yaml, lerna, nx) are descended into rather than claimed, and a single-package Turbo repo is a leaf. Detector order appends Go then Web (Web last, most permissive). * chore(release): prepare v0.1.0 (#16) Rewrite the README for the released tool (eight stacks, full detect/build/sign/upload pipeline, install via binary or go install, usage and flags). Make Homebrew and Scoop upload skip gracefully when HOMEBREW_TAP_TOKEN is absent, so the release ships binaries and the GitHub release without a token and auto-enables tap publishing once the secret is set. * docs: real install instructions, and bump actions off Node 20 (#18) README now shows the working Homebrew cask and Scoop bucket commands published by the v0.1.0 release, and notes that go install lands in GOPATH/bin. Mark 0.1.0 as released in the changelog instead of leaving shipped work under Unreleased. Bump actions/checkout to v7 and actions/setup-go to v7 across CI and release, clearing the Node 20 deprecation warning on the release run.
1 participant
What
Milestone 3: guided release signing.
anvil sign(andanvil build --sign) add a Sign phase after Build.internal/sign: generates a PKCS12 keystore (keytool), writes key.properties, wires Gradle signingConfigs, writes an iOS ExportOptions.plist, and appends signing secrets to .gitignore.flutter build ipa --export-options-plist; React Native and native iOS usexcodebuild archivethen-exportArchive. Android signs at build time via the wired Gradle signingConfig.ANVIL_STORE_PASS,ANVIL_TEAM_ID), never committed.--dry-runmakes no changes.Verification
./checkand staticcheck green. Coverage: sign 85%, detect 89%, pipeline 86%.anvil sign --dry-runandanvil build --sign --dry-runproduce no side effects (verified after fixing a dry-run bug found in review).Scope and deferrals
MVP is Android keystore + Gradle signing and iOS automatic-signing dev/ad-hoc export (macOS, existing Apple identity). Deferred (issues): App Store export, App Store Connect API and fastlane match, OS keychain storage, Play App Signing enrollment, and apksigner signing of a loose prebuilt APK.
Summary by CodeRabbit
New Features
anvil sign.anvil build --signto configure signing during builds.--dry-runplanning without modifying project files.Documentation