extract Fastlane - #6788
extract Fastlane#6788tobiasKaminsky wants to merge 1 commit into
Conversation
Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughThe changes add Talk app and repository identifiers to Priority: ⬇️ Low Merge Risk: 🟡 Moderate · up to Fastlane commands cannot load this configuration because its imported shared file is missing. Provide that file or correct the import before merging; no repository-controlled workflow was found invoking the removed alpha lane. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The new release setup depends on a shared configuration file that is not included here. Its signing and publishing safeguards cannot be verified from this change. No exploitable security issue is established. Retained concerns
Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description includes the required template sections and links to the related pull request, but it provides no substantive change summary. The screenshots remain placeholders, the TODO is unchanged, and all checklist items are unchecked.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 2a74fabf-860b-4bc2-9633-42eeb96a310f
📒 Files selected for processing (2)
.gitignorefastlane/Fastfile
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
| GRADLE_SIGNING_PROPERTIES = { | ||
| "android.injected.signing.store.file" => ENV["FASTLANE_TALK_UPLOAD_STORE_FILE"], | ||
| "android.injected.signing.store.password" => ENV["FASTLANE_TALK_UPLOAD_STORE_PASSWORD"], | ||
| "android.injected.signing.key.alias" => ENV["FASTLANE_TALK_UPLOAD_KEY_ALIAS"], | ||
| "android.injected.signing.key.password" => ENV["FASTLANE_TALK_UPLOAD_KEY_PASSWORD"], | ||
| }.freeze |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Validate the signing environment variables before use.
GRADLE_SIGNING_PROPERTIES reads four ENV values with no presence check. If a variable is unset, the value is nil. Gradle then receives a missing signing property. The build can fail late with an unclear error. It can also produce an unsigned or wrongly signed artifact.
Fail fast if a value is missing or empty. Use ENV.fetch with a check, or UI.user_error!.
Proposed fix
+SIGNING_ENV_KEYS = %w[
+ FASTLANE_TALK_UPLOAD_STORE_FILE
+ FASTLANE_TALK_UPLOAD_STORE_PASSWORD
+ FASTLANE_TALK_UPLOAD_KEY_ALIAS
+ FASTLANE_TALK_UPLOAD_KEY_PASSWORD
+].freeze
+
+missing = SIGNING_ENV_KEYS.select { |k| ENV[k].to_s.empty? }
+UI.user_error!("Missing env vars: #{missing.join(', ')}") unless missing.empty?
+
GRADLE_SIGNING_PROPERTIES = {If these values are only needed by some lanes, run the check inside those lanes instead of at load time. The load-time check would also break lanes that need no signing.
Based on learnings: validate that security-sensitive values from environment variables are present and non-empty, and fail fast if they are missing.
📝 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.
| GRADLE_SIGNING_PROPERTIES = { | |
| "android.injected.signing.store.file" => ENV["FASTLANE_TALK_UPLOAD_STORE_FILE"], | |
| "android.injected.signing.store.password" => ENV["FASTLANE_TALK_UPLOAD_STORE_PASSWORD"], | |
| "android.injected.signing.key.alias" => ENV["FASTLANE_TALK_UPLOAD_KEY_ALIAS"], | |
| "android.injected.signing.key.password" => ENV["FASTLANE_TALK_UPLOAD_KEY_PASSWORD"], | |
| }.freeze | |
| SIGNING_ENV_KEYS = %w[ | |
| FASTLANE_TALK_UPLOAD_STORE_FILE | |
| FASTLANE_TALK_UPLOAD_STORE_PASSWORD | |
| FASTLANE_TALK_UPLOAD_KEY_ALIAS | |
| FASTLANE_TALK_UPLOAD_KEY_PASSWORD | |
| ].freeze | |
| missing = SIGNING_ENV_KEYS.select { |k| ENV[k].to_s.empty? } | |
| UI.user_error!("Missing env vars: #{missing.join(', ')}") unless missing.empty? | |
| GRADLE_SIGNING_PROPERTIES = { | |
| "android.injected.signing.store.file" => ENV["FASTLANE_TALK_UPLOAD_STORE_FILE"], | |
| "android.injected.signing.store.password" => ENV["FASTLANE_TALK_UPLOAD_STORE_PASSWORD"], | |
| "android.injected.signing.key.alias" => ENV["FASTLANE_TALK_UPLOAD_KEY_ALIAS"], | |
| "android.injected.signing.key.password" => ENV["FASTLANE_TALK_UPLOAD_KEY_PASSWORD"], | |
| }.freeze |
Source: Learnings
📱 QA build
The QA build installs alongside a released Nextcloud app, so you can keep Downloading the file requires a GitHub account, so open this link on the |
mahibi
left a comment
mahibi
left a comment
There was a problem hiding this comment.
up to you @tobiasKaminsky if the "Fail fast" suggestion should be implemented here or if you plan to handle this in the config repo?
| GRADLE_SIGNING_PROPERTIES = { | ||
| "android.injected.signing.store.file" => ENV["FASTLANE_TALK_UPLOAD_STORE_FILE"], | ||
| "android.injected.signing.store.password" => ENV["FASTLANE_TALK_UPLOAD_STORE_PASSWORD"], | ||
| "android.injected.signing.key.alias" => ENV["FASTLANE_TALK_UPLOAD_KEY_ALIAS"], | ||
| "android.injected.signing.key.password" => ENV["FASTLANE_TALK_UPLOAD_KEY_PASSWORD"], | ||
| }.freeze |
2 participants
Part of nextcloud/android-config#391
🖼️ Screenshots
🚧 TODO
🏁 Checklist
/backport to stable-xx.x🤖 AI (if applicable)