Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
115 changes: 68 additions & 47 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,53 +37,74 @@ jobs:
- name: Run tests
run: composer run phpunit -- --no-coverage

# This does not work
# behat:
# name: Tests (Behat with PHP ${{ matrix.php }})
# runs-on: ubuntu-latest

# services:
# redmine-6-0:
# image: redmine:6.0.7
# ports:
# - "5060:3000"
# env:
# # Workaround: Remove secret for Rails 7.2 so it will be generated automatically
# # @see https://github.com/docker-library/redmine/issues/349#issuecomment-2516634932
# # REDMINE_SECRET_KEY_BASE: supersecretkey
# REDMINE_PLUGINS_MIGRATE: true
# volumes:
# - /home/runner/work/_temp/redmine-60007_data/files:/usr/src/redmine/files
# - /home/runner/work/_temp/redmine-60007_data/sqlite:/usr/src/redmine/sqlite
# options: --health-cmd="wget -O /dev/null http://localhost:3000" --health-start-period=30s --health-interval=30s --health-timeout=30s --health-retries=3

# strategy:
# fail-fast: false
# matrix:
# operating-system: ["ubuntu-latest"]
# php: ["8.3"]

# steps:
# - name: Checkout
# uses: actions/checkout@v4
# with:
# fetch-depth: 2

# - name: Setup PHP, with composer and extensions
# uses: shivammathur/setup-php@v2 #https://github.com/shivammathur/setup-php
# with:
# php-version: ${{ matrix.php }}
# tools: phpunit
# extensions: mbstring, xml, ctype, iconv, intl, pdo_sqlite
# coverage: xdebug

# # Install composer dependencies and handle caching in one go.
# # @link https://github.com/marketplace/actions/install-composer-dependencies
# - name: "Install Composer dependencies"
# uses: "ramsey/composer-install@v2"

# - name: Run behat
# run: vendor/bin/behat --config tests/Behat/behat.yml --profile=github-actions --suite=redmine_6_0
behat:
name: Tests (Behat with PHP ${{ matrix.php }})
runs-on: ubuntu-latest
Comment on lines +40 to +42
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Restrict token scope for the new behat job.

Add a permissions block with least privilege so this job doesn’t inherit broad default GITHUB_TOKEN permissions.

🔧 Proposed fix
   behat:
     name: Tests (Behat with PHP ${{ matrix.php }})
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
📝 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
behat:
name: Tests (Behat with PHP ${{ matrix.php }})
runs-on: ubuntu-latest
behat:
name: Tests (Behat with PHP ${{ matrix.php }})
runs-on: ubuntu-latest
permissions:
contents: read
🧰 Tools
🪛 zizmor (1.25.2)

[warning] 40-107: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block

(excessive-permissions)

🤖 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 @.github/workflows/tests.yml around lines 40 - 42, The behat job inherits
broad GITHUB_TOKEN permissions; restrict it by adding a permissions block under
the job named "behat" that grants least privilege (e.g., at minimum set
contents: read and any other specific read-only scopes your tests require) so
the job no longer inherits wide default permissions from the workflow root;
update the job definition in the .github/workflows/tests.yml for the behat job
to include this permissions block.


strategy:
fail-fast: false
matrix:
operating-system: ["ubuntu-latest"]
php: ["8.3"]

steps:
- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 2

- name: Setup PHP, with composer and extensions
uses: shivammathur/setup-php@v2
with:
php-version: ${{ matrix.php }}
tools: phpunit
extensions: mbstring, xml, ctype, iconv, intl, pdo_sqlite, curl
coverage: xdebug

- name: Install Composer dependencies
uses: ramsey/composer-install@v2
Comment on lines +51 to +65
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file=".github/workflows/tests.yml"

echo "Potentially unpinned uses: references (not full 40-char SHA):"
rg -n '^\s*uses:\s*[^@]+@(?![0-9a-fA-F]{40}\b).+$' "$file" || true

echo
echo "Checkout steps missing persist-credentials: false (manual check context shown):"
rg -n -A6 -B1 'uses:\s*actions/checkout@' "$file"

Repository: kbsali/php-redmine-api

Length of output: 1278


🏁 Script executed:

#!/bin/bash
set -euo pipefail

file=".github/workflows/tests.yml"

echo "=== All 'uses:' references in the file ==="
rg -n 'uses:' "$file"

echo
echo "=== Lines 51-65 (target of review) ==="
sed -n '51,65p' "$file" | cat -n

echo
echo "=== Count of unpinned vs pinned actions ==="
echo "Using version tags (unpinned):"
rg 'uses:.*`@v`[0-9]' "$file" | wc -l

echo "Using full SHAs (pinned):"
rg 'uses:.*@[0-9a-fA-F]{40}' "$file" | wc -l

Repository: kbsali/php-redmine-api

Length of output: 1328


🌐 Web query:

GitHub Actions security best practices pin action SHA commit credentials

💡 Result:

Pinning GitHub Actions to a full-length commit SHA is a critical security best practice to prevent supply chain attacks, such as tag hijacking or malicious code injection [1][2][3]. Why Pinning to SHA is Required GitHub Actions referenced by tags (e.g., @v1) or branches (e.g., @main) are mutable, meaning the underlying code can be changed by the action maintainer or an attacker who gains access to the repository [2][3][4]. Pinning to a full-length (40-character) commit SHA ensures that your workflow always executes the exact, immutable version of the code you have verified [2][3][5]. This is currently the only way to use an action as an immutable release [6][7][3]. How to Pin Actions To pin an action, replace the tag or branch reference with the full-length commit SHA of the desired version [2][3]. Example: uses: actions/checkout@v4 # Vulnerable to tag manipulation uses: actions/checkout@11bd719 # Secure, pinned to SHA Best Practices for Maintenance 1. Use Version Comments: Always include the original version tag in a comment next to the SHA to make it easier to identify which version is being used and to facilitate manual updates [1][8][3]. 2. Automate Updates: Because SHA pinning requires manual updates to receive bug fixes and security patches, use tools like Dependabot or Renovate to automatically propose updates to your pinned SHAs [1][8][3][5]. 3. Verify Source: Ensure the SHA originates from the official action repository and not a fork [9][6][3]. 4. Enforce via Policy: Organizations can enforce this practice at the repository or organization level by enabling the "Require actions to be pinned to a full-length commit SHA" policy in GitHub Actions settings [6][10][11]. When enabled, any workflow using a non-SHA reference will fail [10][4][11]. 5. Use Security Auditing Tools: Tools like zizmor can automatically detect unpinned action references in your workflows [8][12][13]. Regarding Credentials While SHA pinning protects the integrity of the action code itself, it does not inherently protect against credential theft if the action is malicious or compromised [14]. Always follow the principle of least privilege by: - Setting default GITHUB_TOKEN permissions to read-only [15][7]. - Using OpenID Connect (OIDC) to access cloud resources instead of storing long-term secrets in GitHub [16]. - Auditing the permissions and network access required by third-party actions [15][17].

Citations:


🌐 Web query:

GitHub checkout action persist-credentials false security best practice

💡 Result:

Setting persist-credentials: false in the actions/checkout GitHub Action is considered a security best practice to minimize the risk of credential exposure [1][2]. By default, actions/checkout configures the local Git environment with the authentication token (such as the GITHUB_TOKEN) used to clone the repository [3][4]. This makes the token available to any subsequent steps or scripts running within the same job, as they can read it from the Git configuration [5][2]. If a workflow step is compromised or if the repository is inadvertently archived and uploaded as an artifact, this token could be exposed [6][2]. To mitigate this risk, you should explicitly disable this behavior when the token is not required for subsequent Git operations [1]: - uses: actions/checkout@v4 with: persist-credentials: false While recent updates to actions/checkout have improved security by moving these credentials to a separate file in $RUNNER_TEMP rather than directly in.git/config [3][7], setting persist-credentials: false remains the recommended approach to follow the principle of least privilege and prevent unnecessary credential persistence [1][2]. If your workflow does not need to perform authenticated Git operations (like git push or git submodule updates) after the initial checkout, there is no reason to keep the credentials active [1].

Citations:


Pin action references to full commit SHAs and add persist-credentials: false to checkout.

All actions in this workflow use version tags (@v4, @v2, etc.) instead of full 40-character commit SHAs, creating supply-chain attack risk. Additionally, the checkout step should set persist-credentials: false to avoid unnecessarily exposing credentials to subsequent steps.

🧰 Tools
🪛 zizmor (1.25.2)

[warning] 51-54: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)


[error] 52-52: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)


[error] 57-57: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)


[error] 65-65: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🤖 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 @.github/workflows/tests.yml around lines 51 - 65, The workflow uses floating
tags for actions (actions/checkout@v4, shivammathur/setup-php@v2,
ramsey/composer-install@v2) and the checkout step lacks persist-credentials:
false; fix by replacing each action tag with its corresponding full 40-character
commit SHA for supply-chain safety and add persist-credentials: false under the
Checkout step (the actions/checkout usage) to prevent passing repo credentials
to later steps; ensure the SHAs are verified from each action's GitHub
repository and update the workflow accordingly.


- name: Create data directories
run: |
mkdir -p .docker/redmine-60101_data/{files,sqlite}
mkdir -p .docker/redmine-60008_data/{files,sqlite}
mkdir -p .docker/redmine-50111_data/{files,sqlite}
chmod -R 777 .docker/redmine-*_data/

- name: Start Redmine containers
run: docker compose up -d redmine-6-1 redmine-6-0 redmine-5-1

- name: Wait for Redmine instances to be healthy
run: |
for i in $(seq 1 60); do
if curl -sf http://localhost:5061 > /dev/null 2>&1 && \
curl -sf http://localhost:5060 > /dev/null 2>&1 && \
curl -sf http://localhost:5051 > /dev/null 2>&1; then
echo "All Redmine instances are ready"
exit 0
fi
echo "Waiting for Redmine instances... ($i/60)"
sleep 5
done
echo "Redmine instances did not become ready in time"
docker compose logs redmine-6-1 redmine-6-0 redmine-5-1
exit 1

- name: Fix permissions
run: sudo chmod -R 777 .docker/redmine-*_data/

- name: Run behat [Redmine 6.1]
run: vendor/bin/behat --config tests/Behat/behat.yml --profile=github-actions --format=progress --suite=redmine_6_1

- name: Run behat [Redmine 6.0]
run: vendor/bin/behat --config tests/Behat/behat.yml --profile=github-actions --format=progress --suite=redmine_6_0

- name: Run behat [Redmine 5.1]
run: vendor/bin/behat --config tests/Behat/behat.yml --profile=github-actions --format=progress --suite=redmine_5_1

- name: Cleanup
if: always()
run: docker compose down -v

code-quality:
name: Check ${{ matrix.tool }} (PHP ${{ matrix.php }})
Expand Down
4 changes: 2 additions & 2 deletions tests/Behat/Bootstrap/FeatureContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,15 @@ public static function clean(AfterSuiteScope $scope): void
*/
private array $lastReturnAsArray;

public function __construct(string $redmineVersion, string $rootPath)
public function __construct(string $redmineVersion, string $rootPath, ?string $redmineUrl = null)
{
$version = RedmineVersion::tryFrom($redmineVersion);

if (!$version instanceof RedmineVersion) {
throw new InvalidArgumentException('Redmine ' . $redmineVersion . ' is not supported.');
}

$this->redmine = self::$tracer::getRedmineInstance($version, $rootPath);
$this->redmine = self::$tracer::getRedmineInstance($version, $rootPath, $redmineUrl);
}

/**
Expand Down
9 changes: 6 additions & 3 deletions tests/Behat/behat.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,17 @@ github-actions:
contexts:
- Redmine\Tests\Behat\Bootstrap\FeatureContext:
redmineVersion: '6.1.1'
rootPath: '/home/runner/work/_temp'
rootPath: '%paths.base%/../../.docker'
redmineUrl: 'http://localhost:5061'
redmine_6_0:
contexts:
- Redmine\Tests\Behat\Bootstrap\FeatureContext:
redmineVersion: '6.0.8'
rootPath: '/home/runner/work/_temp'
rootPath: '%paths.base%/../../.docker'
redmineUrl: 'http://localhost:5060'
redmine_5_1:
contexts:
- Redmine\Tests\Behat\Bootstrap\FeatureContext:
redmineVersion: '5.1.11'
rootPath: '/home/runner/work/_temp'
rootPath: '%paths.base%/../../.docker'
redmineUrl: 'http://localhost:5051'
4 changes: 2 additions & 2 deletions tests/RedmineExtension/BehatHookTracer.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ final class BehatHookTracer implements InstanceRegistration
*/
private static array $instances = [];

public static function getRedmineInstance(RedmineVersion $redmineVersion, string $rootPath): RedmineInstance
public static function getRedmineInstance(RedmineVersion $redmineVersion, string $rootPath, ?string $redmineUrl = null): RedmineInstance
{
if (!self::$tracer instanceof \Redmine\Tests\RedmineExtension\BehatHookTracer) {
throw new RuntimeException('You can only get a Redmine instance while a Behat Suite is running.');
}

if (! array_key_exists($redmineVersion->asId(), self::$instances)) {
RedmineInstance::create(self::$tracer, $redmineVersion, $rootPath);
RedmineInstance::create(self::$tracer, $redmineVersion, $rootPath, $redmineUrl);
}

return self::$instances[$redmineVersion->asId()];
Expand Down
8 changes: 4 additions & 4 deletions tests/RedmineExtension/RedmineInstance.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ final class RedmineInstance
/**
* @param InstanceRegistration $tracer Required to ensure that RedmineInstance is created while Test Runner is running
*/
public static function create(InstanceRegistration $tracer, RedmineVersion $version, string $rootPath): void
public static function create(InstanceRegistration $tracer, RedmineVersion $version, string $rootPath, ?string $redmineUrl = null): void
{
$tracer->registerInstance(new self($tracer, $version, $rootPath));
$tracer->registerInstance(new self($tracer, $version, $rootPath, $redmineUrl));
}

private InstanceRegistration $tracer;
Expand All @@ -41,7 +41,7 @@ public static function create(InstanceRegistration $tracer, RedmineVersion $vers

private string $apiKey;

private function __construct(InstanceRegistration $tracer, RedmineVersion $version, string $rootPath)
private function __construct(InstanceRegistration $tracer, RedmineVersion $version, string $rootPath, ?string $redmineUrl = null)
{
$this->tracer = $tracer;
$this->version = $version;
Expand All @@ -65,7 +65,7 @@ private function __construct(InstanceRegistration $tracer, RedmineVersion $versi

$parts = explode('.', $version->asString());

$this->redmineUrl = 'http://redmine-' . intval($parts[0]) . '-' . intval($parts[1]) . ':3000';
$this->redmineUrl = $redmineUrl ?? ('http://redmine-' . intval($parts[0]) . '-' . intval($parts[1]) . ':3000');
$this->apiKey = sha1($versionId . time());

$this->runHealthChecks($version);
Expand Down
Loading