Skip to content

fix: CI for signing jars #1059

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
kolipakakondal wants to merge 5 commits into
base: master
Choose a base branch
from
Open

fix: CI for signing jars #1059

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
729c4eb
fix: sign jars to avoid notorization issues
kolipakakondal Oct 4, 2024
777aef2
Update signjars.yml
kolipakakondal Oct 5, 2024
b971bd5
Update signjars.yml
kolipakakondal Oct 7, 2024
45eea31
Update signjars.yml
kolipakakondal Oct 7, 2024
cf63e72
Update signjars.yml
kolipakakondal Oct 7, 2024
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
56 changes: 56 additions & 0 deletions .github/workflows/signjars.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
name: Java CI with Maven

on:
push:
branches: [ master ]
pull_request:
branches: [ master ]

jobs:
build:

runs-on: macos-latest

steps:
- uses: actions/checkout@v3

- name: Set up JDK 17
uses: actions/setup-java@v3
with:
java-version: '17'
distribution: 'temurin'

- name: Sign JARs
run: |
# Export secrets as environment variables
export JARSIGNER_KEYSTORE_B64=${{ secrets.JARSIGNER_REL_KEYSTORE_B64 }}
export JARSIGNER_STOREPASS=${{ secrets.JARSIGNER_REL_STOREPASS }}
export JARSIGNER_ALIAS=${{ secrets.JARSIGNER_REL_ALIAS }}

# Set up the keystore file path
KEYSTORE_FILE="${PWD}/{{secrets.JARSIGNER_KEYSTORE}}"
echo "Keystore file: ${KEYSTORE_FILE}"

# Decode and save the base64-encoded keystore to the file
printf "%s" "${JARSIGNER_KEYSTORE_B64}" | base64 -d > "${KEYSTORE_FILE}"

# Sign all JAR files located in the specified directory
LIB_DIR="${PWD}/BUNDLES/com.espressif.idf.serial.monitor/lib"
echo "Signing JAR files in ${LIB_DIR}"
for jar in "${LIB_DIR}"/*.jar; do
echo "Signing JAR file: ${jar}"
jarsigner -keystore "${KEYSTORE_FILE}" \
-storepass "${JARSIGNER_STOREPASS}" \
-signedjar "${jar}" \
"${jar}" "${JARSIGNER_ALIAS}"
done
Comment on lines +36 to +78
Copy link

@coderabbitai coderabbitai bot Oct 5, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Optimize JAR processing and signing

Consider the following improvements to enhance performance, reliability, and security:

  1. Use parallel processing for signing multiple JARs.
  2. Add error checking and logging for each step.
  3. Use read -r to prevent backslash mangling in filenames.
  4. Quote variables to prevent word splitting and globbing.

Here's an optimized version:

export -f codesign_and_verify
find "$LIB_DIR" -name "*.jar" -print0 | parallel -0 process_jar {}

process_jar() {
  local jar_file="$1"
  echo "Processing JAR file: $jar_file"
  local temp_dir
  temp_dir=$(mktemp -d) || { echo "Failed to create temp directory"; return 1; }
  
  if ! unzip -q "$jar_file" -d "$temp_dir"; then
    echo "Error extracting $jar_file"
    rm -rf "$temp_dir"
    return 1
  fi

  find "$temp_dir" \( -name "*.jnilib" -o -name "*.dylib" \) -print0 | 
    xargs -0 -I {} bash -c 'codesign_and_verify "$@"' _ {} "$PWD/releng/com.espressif.idf.product/entitlements/espressif-ide.entitlement"

  if ! (cd "$temp_dir" && zip -qr "$jar_file" .); then
    echo "Error repackaging $jar_file"
    rm -rf "$temp_dir"
    return 1
  fi

  if ! codesign_and_verify "$jar_file" "$PWD/releng/com.espressif.idf.product/entitlements/espressif-ide.entitlement"; then
    echo "Error signing JAR $jar_file"
    rm -rf "$temp_dir"
    return 1
  fi

  rm -rf "$temp_dir"
  echo "Successfully processed $jar_file"
}

codesign_and_verify() {
  local file="$1"
  local entitlements="$2"
  echo "Signing file: $file"
  if ! /usr/bin/codesign --entitlements "$entitlements" --options runtime --force -s "ESPRESSIF SYSTEMS (SHANGHAI) CO., LTD. (QWXF6GB4AV)" --timestamp --deep "$file"; then
    echo "Error signing $file"
    return 1
  fi
  if ! /usr/bin/codesign --verify --deep --strict --verbose=2 "$file"; then
    echo "Verification failed for $file"
    return 1
  fi
  echo "Successfully signed and verified $file"
}

This version uses GNU Parallel for concurrent processing, adds error checking, uses read -r, and quotes variables for improved security and reliability.

Comment on lines +42 to +78
Copy link

@coderabbitai coderabbitai bot Oct 7, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve bash script for better error handling and security.

Consider the following improvements to the JAR signing process:

  1. Use set -euo pipefail at the beginning of the script for better error handling.
  2. Use read -r to prevent backslash interpretation in filenames.
  3. Quote variables to prevent word splitting and globbing.
  4. Use find with -print0 and xargs -0 for better handling of filenames with spaces.

Here's an example of how to improve the script:

set -euo pipefail

for jar in "${LIB_DIR}"/*.jar; do
  if [ ! -f "$jar" ]; then
    echo "JAR file not found: ${jar}"
    continue
  fi

  TEMP_DIR=$(mktemp -d)
  trap 'rm -rf "$TEMP_DIR"' EXIT

  unzip -q "$jar" -d "$TEMP_DIR"

  find "$TEMP_DIR" \( -name "*.jnilib" -o -name "*.dylib" \) -print0 | 
    xargs -0 -I {} /usr/bin/codesign -vvvv --entitlements "$PWD/releng/com.espressif.idf.product/entitlements/espressif-ide.entitlement" --options runtime --force -s "ESPRESSIF SYSTEMS (SHANGHAI) CO., LTD. (QWXF6GB4AV)" --timestamp --deep "{}"

  (
    cd "$TEMP_DIR" && 
    zip -r "${SIGNED_JARS_DIR}/$(basename "$jar")" .
  )

  /usr/bin/codesign -vvvv --entitlements "$PWD/releng/com.espressif.idf.product/entitlements/espressif-ide.entitlement" --force --deep --options runtime --timestamp -s "ESPRESSIF SYSTEMS (SHANGHAI) CO., LTD. (QWXF6GB4AV)" "${SIGNED_JARS_DIR}/$(basename "$jar")"

  /usr/bin/codesign -dvv "${SIGNED_JARS_DIR}/$(basename "$jar")"
done

These changes will make the script more robust and secure.

🧰 Tools
🪛 yamllint

[error] 75-75: trailing spaces

(trailing-spaces)

Comment on lines +36 to +78
Copy link

@coderabbitai coderabbitai bot Oct 7, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Optimize JAR processing and signing.

The current implementation works, but it can be improved for better performance, reliability, and security:

  1. Use parallel processing for signing multiple JARs.
  2. Add error checking and logging for each step.
  3. Use read -r to prevent backslash mangling in filenames.
  4. Quote variables to prevent word splitting and globbing.

Here's an optimized version of the JAR processing logic:

export -f codesign_and_verify
find "$LIB_DIR" -name "*.jar" -print0 | parallel -0 process_jar {}

process_jar() {
  local jar_file="$1"
  echo "Processing JAR file: $jar_file"
  local temp_dir
  temp_dir=$(mktemp -d) || { echo "Failed to create temp directory"; return 1; }
  
  if ! unzip -q "$jar_file" -d "$temp_dir"; then
    echo "Error extracting $jar_file"
    rm -rf "$temp_dir"
    return 1
  fi

  find "$temp_dir" \( -name "*.jnilib" -o -name "*.dylib" \) -print0 | 
    xargs -0 -I {} bash -c 'codesign_and_verify "$@"' _ {} "$PWD/releng/com.espressif.idf.product/entitlements/espressif-ide.entitlement"

  if ! (cd "$temp_dir" && zip -qr "$jar_file" .); then
    echo "Error repackaging $jar_file"
    rm -rf "$temp_dir"
    return 1
  fi

  if ! codesign_and_verify "$jar_file" "$PWD/releng/com.espressif.idf.product/entitlements/espressif-ide.entitlement"; then
    echo "Error signing JAR $jar_file"
    rm -rf "$temp_dir"
    return 1
  fi

  rm -rf "$temp_dir"
  echo "Successfully processed $jar_file"
}

codesign_and_verify() {
  local file="$1"
  local entitlements="$2"
  echo "Signing file: $file"
  if ! /usr/bin/codesign --entitlements "$entitlements" --options runtime --force -s "ESPRESSIF SYSTEMS (SHANGHAI) CO., LTD. (QWXF6GB4AV)" --timestamp --deep "$file"; then
    echo "Error signing $file"
    return 1
  fi
  if ! /usr/bin/codesign --verify --deep --strict --verbose=2 "$file"; then
    echo "Verification failed for $file"
    return 1
  fi
  echo "Successfully signed and verified $file"
}

This version uses GNU Parallel for concurrent processing, adds error checking, uses read -r, and quotes variables for improved security and reliability.


# Clean up the keystore file
rm -v "${KEYSTORE_FILE}"
Copy link

@coderabbitai coderabbitai bot Oct 4, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider making the JAR directory configurable

The current implementation uses a hardcoded path for the JAR files directory. To improve flexibility and reusability, consider making this path configurable through an environment variable or workflow input.

Here's an example of how you could modify the script to use an environment variable:

-        LIB_DIR="${PWD}/BUNDLES/com.espressif.idf.serial.monitor/lib"
+        LIB_DIR="${JAR_DIRECTORY:-${PWD}/BUNDLES/com.espressif.idf.serial.monitor/lib}"

Then, you can set the JAR_DIRECTORY environment variable in the workflow or allow it to fall back to the default path.

📝 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.

Suggested change
- name: Sign JARs
run: |
# Export secrets as environment variables
export JARSIGNER_KEYSTORE_B64=${{ secrets.JARSIGNER_REL_KEYSTORE_B64 }}
export JARSIGNER_STOREPASS=${{ secrets.JARSIGNER_REL_STOREPASS }}
export JARSIGNER_ALIAS=${{ secrets.JARSIGNER_REL_ALIAS }}
# Set up the keystore file path
KEYSTORE_FILE="${PWD}/{{secrets.JARSIGNER_KEYSTORE}}"
echo "Keystore file: ${KEYSTORE_FILE}"
# Decode and save the base64-encoded keystore to the file
printf "%s" "${JARSIGNER_KEYSTORE_B64}" | base64 -d > "${KEYSTORE_FILE}"
# Sign all JAR files located in the specified directory
LIB_DIR="${PWD}/BUNDLES/com.espressif.idf.serial.monitor/lib"
echo "Signing JAR files in ${LIB_DIR}"
for jar in "${LIB_DIR}"/*.jar; do
echo "Signing JAR file: ${jar}"
jarsigner -keystore "${KEYSTORE_FILE}" \
-storepass "${JARSIGNER_STOREPASS}" \
-signedjar "${jar}" \
"${jar}" "${JARSIGNER_ALIAS}"
done
# Clean up the keystore file
rm -v "${KEYSTORE_FILE}"
- name: Sign JARs
run: |
# Export secrets as environment variables
export JARSIGNER_KEYSTORE_B64=${{ secrets.JARSIGNER_REL_KEYSTORE_B64 }}
export JARSIGNER_STOREPASS=${{ secrets.JARSIGNER_REL_STOREPASS }}
export JARSIGNER_ALIAS=${{ secrets.JARSIGNER_REL_ALIAS }}
# Set up the keystore file path
KEYSTORE_FILE="${PWD}/{{secrets.JARSIGNER_KEYSTORE}}"
echo "Keystore file: ${KEYSTORE_FILE}"
# Decode and save the base64-encoded keystore to the file
printf "%s" "${JARSIGNER_KEYSTORE_B64}" | base64 -d > "${KEYSTORE_FILE}"
# Sign all JAR files located in the specified directory
LIB_DIR="${JAR_DIRECTORY:-${PWD}/BUNDLES/com.espressif.idf.serial.monitor/lib}"
echo "Signing JAR files in ${LIB_DIR}"
for jar in "${LIB_DIR}"/*.jar; do
echo "Signing JAR file: ${jar}"
jarsigner -keystore "${KEYSTORE_FILE}" \
-storepass "${JARSIGNER_STOREPASS}" \
-signedjar "${jar}" \
"${jar}" "${JARSIGNER_ALIAS}"
done
# Clean up the keystore file
rm -v "${KEYSTORE_FILE}"

Copy link

@coderabbitai coderabbitai bot Oct 4, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding error handling and logging.

While the current implementation works, it could benefit from improved error handling and logging:

  1. Check if the JAR directory exists before attempting to sign files.
  2. Add error handling for the jarsigner command.
  3. Implement more verbose logging to aid in troubleshooting.

Here's an example of how you could enhance the script:

echo "Signing JAR files in ${LIB_DIR}"
if [ ! -d "${LIB_DIR}" ]; then
  echo "Error: Directory ${LIB_DIR} does not exist."
  exit 1
fi

for jar in "${LIB_DIR}"/*.jar; do
  if [ -f "$jar" ]; then
    echo "Signing JAR file: ${jar}"
    if jarsigner -keystore "${KEYSTORE_FILE}" \
                 -storepass "${JARSIGNER_STOREPASS}" \
                 -signedjar "${jar}" \
                 "${jar}" "${JARSIGNER_ALIAS}"; then
      echo "Successfully signed ${jar}"
    else
      echo "Error: Failed to sign ${jar}"
      exit 1
    fi
  fi
done

echo "All JAR files signed successfully"

This enhancement adds checks for the directory and file existence, error handling for the jarsigner command, and more detailed logging.


- name: Upload Signed JAR Files
if: ${{ !cancelled() }}
uses: actions/upload-artifact@v4
with:
name: signed-jar-files
path: BUNDLES/com.espressif.idf.serial.monitor/lib/*.jar