Repository signing: add support for dual signing #8320
Conversation
|
""" WalkthroughThe publishing workflow in the repository tool was updated to add the Suggested labels
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
08
![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 details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tools/repository/repo(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Shell script analysis
🔇 Additional comments (2)
tools/repository/repo (2)
137-137: LGTM: Correctly disables automatic signing for dual signing approach.The
-skip-signingflag appropriately prevents aptly from automatically signing during the publish step, allowing the new manual signing process to handle dual key signing.
277-277: LGTM: Correctly integrates dual signing into the update workflow.The signing function call is properly placed after publishing and uses consistent parameter passing.
tools/repository/repo
Outdated
| # signing repository | ||
| # | ||
| # $1: Input folder | ||
| # $2: Output folder | ||
| # $3: Command | ||
| # $4: GPG password | ||
| # $5: jammy,sid | ||
|
|
||
| signing(){ | ||
| GPG_PARAMETERS=( | ||
| "--yes" | ||
| "--armor" | ||
| "-u" "3CD6F4E701E8E7F91468ABCEAC67DF56D680226A" # Igor Pecovnik (Ljubljana, Slovenia) <igor.pecovnik@gmail.com> | ||
| "-u" "8CFA83D13EB2181EEF5843E41EB30FAF236099FE" # Armbian Repository Signing Key (Repository Key) <info@armbian.com> | ||
| ) | ||
| for i in "$2"/public/dists/*/Release | ||
| do | ||
| DISTRO_PATH="$(dirname "$i")" | ||
| echo $DISTRO_PATH | ||
| gpg "${GPG_PARAMETERS[@]}" --clear-sign -o "$DISTRO_PATH/InRelease" "$i" | ||
| gpg "${GPG_PARAMETERS[@]}" --detach-sign -o "$DISTRO_PATH/Release.gpg" "$i" | ||
| done | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Address security and error handling concerns in the signing function.
The signing function has several areas that need attention:
-
Hardcoded GPG keys: While this may be intentional for this use case, consider making the keys configurable through environment variables or parameters for better security and flexibility.
-
Missing error handling: GPG operations can fail for various reasons (missing keys, wrong passphrase, etc.). Add error checking after each GPG command.
-
Debug output: Line 176 outputs the distro path to stdout, which may clutter the output in production.
Apply this diff to improve error handling and output:
signing(){
GPG_PARAMETERS=(
"--yes"
"--armor"
"-u" "3CD6F4E701E8E7F91468ABCEAC67DF56D680226A" # Igor Pecovnik (Ljubljana, Slovenia) <igor.pecovnik@gmail.com>
"-u" "8CFA83D13EB2181EEF5843E41EB30FAF236099FE" # Armbian Repository Signing Key (Repository Key) <info@armbian.com>
)
for i in "$2"/public/dists/*/Release
do
+ if [[ ! -f "$i" ]]; then
+ echo "Warning: Release file not found: $i" >&2
+ continue
+ fi
DISTRO_PATH="$(dirname "$i")"
- echo $DISTRO_PATH
+ echo "Signing repository: $(basename "$DISTRO_PATH")" | sudo tee -a "${DEBUGFILE}"
- gpg "${GPG_PARAMETERS[@]}" --clear-sign -o "$DISTRO_PATH/InRelease" "$i"
- gpg "${GPG_PARAMETERS[@]}" --detach-sign -o "$DISTRO_PATH/Release.gpg" "$i"
+ if ! gpg "${GPG_PARAMETERS[@]}" --clear-sign -o "$DISTRO_PATH/InRelease" "$i"; then
+ echo "Error: Failed to create InRelease for $DISTRO_PATH" >&2
+ return 1
+ fi
+ if ! gpg "${GPG_PARAMETERS[@]}" --detach-sign -o "$DISTRO_PATH/Release.gpg" "$i"; then
+ echo "Error: Failed to create Release.gpg for $DISTRO_PATH" >&2
+ return 1
+ fi
done
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In tools/repository/repo around lines 158 to 181, the signing function has
hardcoded GPG keys, lacks error handling after GPG commands, and prints debug
output to stdout. To fix this, make the GPG keys configurable via environment
variables or function parameters instead of hardcoding them, add checks after
each gpg command to detect failures and handle errors appropriately (e.g., exit
or log errors), and remove or redirect the echo statement that outputs the
distro path to avoid cluttering production output.
igorpecovnik
force-pushed
the
sign-repo-with-dual-keys
branch
from
96a3468 to
3739ffb
Compare
github-actions
bot
added
size/medium
github-actions
bot
removed
the
Ready to merge
Description
Supporting armbian/apa#18
How Has This Been Tested?
Checklist: