fix: quote APT_SIGNING_KEY_ID in gpg export#1738
Conversation
The unquoted ${APT_SIGNING_KEY_ID:-} on the gpg --export line can cause
word splitting if the key ID contains special characters (unlikely for
hex fingerprints but possible for names/emails). The signing section
above already quotes the same variable correctly via GPG_ARGS — align
the export line with the same pattern.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe apt repository build script now constructs gpg export arguments in an array and conditionally appends a safely quoted signing key id when exporting the public key, writing the result to $OUTPUT_DIR/KEY.gpg. ChangesAPT Repository GPG Key Export
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 1
🤖 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 `@scripts/build-apt-repo.sh`:
- Line 86: The current gpg export command passes an empty quoted
APT_SIGNING_KEY_ID which causes gpg to export nothing when the variable is
unset; change the invocation that writes to KEY.gpg so that APT_SIGNING_KEY_ID
is only added to the --export argument when non-empty (matching the --local-user
conditional pattern used earlier), allowing gpg to use its default key when
APT_SIGNING_KEY_ID is unset.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 80e8c98e-6834-4779-9917-5c9b06ab5b6e
📒 Files selected for processing (1)
scripts/build-apt-repo.sh
|
|
||
| # ── Export public key ───────────────────────────────────────────────────────── | ||
| gpg --batch --yes --armor --export ${APT_SIGNING_KEY_ID:-} > "$OUTPUT_DIR/KEY.gpg" | ||
| gpg --batch --yes --armor --export "${APT_SIGNING_KEY_ID:-}" > "$OUTPUT_DIR/KEY.gpg" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Test gpg export behavior with empty string vs no argument
echo "Testing gpg --export behavior..."
echo "================================"
# Test 1: No argument (exports all keys)
echo -e "\n1. gpg --export (no argument):"
gpg --batch --armor --export 2>&1 | head -n 5
# Test 2: Empty string argument
echo -e "\n2. gpg --export \"\" (empty string):"
gpg --batch --armor --export "" 2>&1 | head -n 5
# Test 3: Show difference in argument passing
echo -e "\n3. Command line argument difference:"
echo " Unquoted \${VAR:-} when empty: arguments disappear"
echo " Quoted \"\${VAR:-}\" when empty: empty string passed"
# Test 4: Check if any keys would be exported
echo -e "\n4. Count of keys in keyring:"
gpg --list-keys --keyid-format LONG 2>&1 | grep -c "^pub" || echo "0"Repository: tinyhumansai/openhuman
Length of output: 463
🏁 Script executed:
cat -n scripts/build-apt-repo.sh | sed -n '60,95p'Repository: tinyhumansai/openhuman
Length of output: 1434
🌐 Web query:
gpg --export empty string argument behavior difference
💡 Result:
Behavior summary (GnuPG): 1) gpg --export "" (empty string as the key specifier) - GnuPG treats the argument to --export as a key specifier (a key ID or any portion of a user ID). If the provided specifier doesn’t match any key/user IDs, no key material is exported and you get “WARNING: nothing exported” [1]. - In practice, “empty string” will not match any key/user ID, so --export "" results in exporting nothing (with the warning) rather than exporting all keys [2][1]. 2) gpg --export with no name given (no argument) - Per the gpg man page, --export with no name given exports all keys from the default keyrings (or those registered via --keyring) [2]. - So the empty-string argument is different from omitting the argument: omitted argument means “all keys”; empty string means “match nothing” (and therefore exports nothing) [2][1]. Notes on quoting: - On the shell, gpg --export "" passes a single empty-string argument. If instead you omit the argument entirely (e.g., just gpg --export), it triggers the “export all keys” behavior [2]. Practical takeaway: - To export all keys: gpg --export > file - To export a specific key/user-id: gpg --export 'some-part-of-uid-or-keyid' - Don’t use an empty-string specifier; it won’t export anything and may produce “WARNING: nothing exported” [2][1].
Citations:
- 1: https://stackoverflow.com/questions/32475460/exporting-public-key-from-gnupg-fails-with-warning-nothing-exported
- 2: https://www.gnupg.org/gph/de/manual/r1023.html
🏁 Script executed:
head -n 20 scripts/build-apt-repo.shRepository: tinyhumansai/openhuman
Length of output: 764
Quoting fix introduces behavioral regression when APT_SIGNING_KEY_ID is unset.
The quoted variable change prevents word-splitting but breaks the documented "default key" behavior. When APT_SIGNING_KEY_ID is unset:
- Before (unquoted):
gpg --export→ exports all keys from keyring (matches--local-userdefault behavior) - After (quoted):
gpg --export ""→ exports nothing with "WARNING: nothing exported"
For consistency with the signing logic (line 73) and the documented behavior, use conditional expansion to only pass the key ID when set:
gpg --batch --yes --armor --export ${APT_SIGNING_KEY_ID:+"$APT_SIGNING_KEY_ID"} > "$OUTPUT_DIR/KEY.gpg"This aligns with the pattern on line 73 where --local-user is only added when the variable is non-empty, allowing gpg to use the default key when unset.
🧰 Tools
🪛 GitHub Actions: PR Quality (soft) / 1_PR Submission Checklist.txt
[error] PR checklist indicates an issue: unquoted ${APT_SIGNING_KEY_ID:-} in the gpg --export line on line 86 may cause word splitting if key IDs contain special characters (inconsistency with quoted usage via GPG_ARGS array at line 73).
🤖 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 `@scripts/build-apt-repo.sh` at line 86, The current gpg export command passes
an empty quoted APT_SIGNING_KEY_ID which causes gpg to export nothing when the
variable is unset; change the invocation that writes to KEY.gpg so that
APT_SIGNING_KEY_ID is only added to the --export argument when non-empty
(matching the --local-user conditional pattern used earlier), allowing gpg to
use its default key when APT_SIGNING_KEY_ID is unset.
Quoting `${APT_SIGNING_KEY_ID:-}` prevented word splitting but introduced
a regression: when the variable is unset, gpg received an empty-string
key specifier and exported nothing ("WARNING: nothing exported") instead
of falling back to the default key. Use the same conditional pattern as
the signing step (line 73) so the argument is omitted entirely when the
variable is empty, restoring default-key behavior.
Addresses @coderabbitai on scripts/build-apt-repo.sh:86.
2 participants
Summary
${APT_SIGNING_KEY_ID:-}in the gpg--exportline ofscripts/build-apt-repo.shContext
The signing section earlier in the same script already quotes this variable correctly via the
GPG_ARGSarray (line 73). The export line on line 86 was the only unquoted usage, creating an inconsistency that could cause word splitting with key IDs containing special characters.Test plan
scripts/build-apt-repo.shwith and withoutAPT_SIGNING_KEY_IDset — export should still work in both cases🤖 Generated with Claude Code
Summary by CodeRabbit