fix(log): Remove api key from logged appsec headers

crowdsecurity · Oct 10, 2024 · b571eea · b571eea
1 parent 2d2f64c
commit b571eea
Show file tree

Hide file tree

Showing 8 changed files with 66 additions and 20 deletions.
diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
@@ -16,29 +16,33 @@ jobs:
     permissions:
       contents: write
 
+    env:
+      TAG_NAME: ${{ github.event.inputs.tag_name }}
+
     steps:
       - name: Check naming convention
         run: |
-          VERIF=$(echo ${{ github.event.inputs.tag_name }} | grep -E "^v([0-9]{1,}\.)([0-9]{1,}\.)([0-9]{1,})(-(alpha|beta)\.[0-9]{1,})?$")
+          VERIF=$(echo ${{ env.TAG_NAME }} | grep -E "^v([0-9]{1,}\.)([0-9]{1,}\.)([0-9]{1,})(-(alpha|beta)\.[0-9]{1,})?$")
           if [ ! ${VERIF} ]
           then
-            echo "Tag name '${{ github.event.inputs.tag_name }}' does not comply with naming convention vX.Y.Z"
+            echo "Tag name '${{ env.TAG_NAME }}' does not comply with naming convention vX.Y.Z"
             exit 1
           fi
 
       - name: Set version number without v
+        id: set-version-number
         run: |
-          echo "VERSION_NUMBER=$(echo ${{ github.event.inputs.tag_name }} | sed 's/v//g' )" >> $GITHUB_ENV
+          echo "version_number=$(echo ${{ env.TAG_NAME }} | sed 's/v//g' )" >> $GITHUB_OUTPUT
 
       - name: Clone sources
         uses: actions/checkout@v4
 
-      - name: Check version ${{ env.VERSION_NUMBER }} consistency in files
+      - name: Check version consistency in files
         # Check src/Constants.php and CHANGELOG.md
         run: |
           # Check public const VERSION = 'vVERSION_NUMBER'; in src/Constants.php
           CONSTANT_VERSION=$(grep -E "public const VERSION = 'v(.*)';"  src/Constants.php | sed 's/ //g')
-          if [[ $CONSTANT_VERSION == "publicconstVERSION='v${{ env.VERSION_NUMBER }}';" ]]
+          if [[ $CONSTANT_VERSION == "publicconstVERSION='${{ env.TAG_NAME }}';" ]]
           then
               echo "CONSTANT VERSION OK"
           else
@@ -51,7 +55,7 @@ jobs:
           echo $CURRENT_DATE
           CHANGELOG_VERSION=$(grep -o -E "## \[(.*)\].* - $CURRENT_DATE"  CHANGELOG.md | head -1 | sed 's/ //g')
           echo $CHANGELOG_VERSION
-          if [[ $CHANGELOG_VERSION == "##[${{ env.VERSION_NUMBER }}]($GITHUB_SERVER_URL/$GITHUB_REPOSITORY/releases/tag/v${{ env.VERSION_NUMBER }})-$CURRENT_DATE" ]]
+          if [[ $CHANGELOG_VERSION == "##[${{ steps.set-version-number.outputs.version_number }}]($GITHUB_SERVER_URL/$GITHUB_REPOSITORY/releases/tag/${{ env.TAG_NAME }})-$CURRENT_DATE" ]]
           then
               echo "CHANGELOG VERSION OK"
           else
@@ -62,39 +66,37 @@ jobs:
           # Check top [_Compare with previous release_](GITHUB_URL/compare/vLAST_TAG...vVERSION_NUMBER) in CHANGELOG.md
           COMPARISON=$(grep -oP "$GITHUB_SERVER_URL/$GITHUB_REPOSITORY/compare/\K(.*)$" CHANGELOG.md | head -1)
           LAST_TAG=$(curl -Ls -o /dev/null -w %{url_effective} $GITHUB_SERVER_URL/$GITHUB_REPOSITORY/releases/latest | grep -oP "\/tag\/\K(.*)$")
-          if [[ $COMPARISON == "$LAST_TAG...v${{ env.VERSION_NUMBER }})" ]]
+          if [[ $COMPARISON == "$LAST_TAG...${{ env.TAG_NAME }})" ]]
           then
             echo "VERSION COMPARISON OK"
           else
             echo "VERSION COMPARISON KO"
             echo $COMPARISON
-            echo "$LAST_TAG...v${{ env.VERSION_NUMBER }})"
+            echo "$LAST_TAG...${{ env.TAG_NAME }})"
             exit 1
           fi
 
-      - name: Create Tag ${{ github.event.inputs.tag_name }}
+      - name: Create Tag
         uses: actions/github-script@v7
         with:
           github-token: ${{ github.token }}
           script: |
             github.rest.git.createRef({
               owner: context.repo.owner,
               repo: context.repo.repo,
-              ref: "refs/tags/${{ github.event.inputs.tag_name }}",
+              ref: "refs/tags/${{ env.TAG_NAME }}",
               sha: context.sha
             })
 
       - name: Prepare release notes
         run: |
           # Retrieve release body and remove ---
-          VERSION_RELEASE_NOTES=$(awk -v ver="[${{ env.VERSION_NUMBER }}]($GITHUB_SERVER_URL/$GITHUB_REPOSITORY/releases/tag/v${{ env.VERSION_NUMBER }})" '/^## / { if (p) { exit }; if ($2 == ver) { p=1; next} } p && NF' CHANGELOG.md | sed ':a;N;$!ba;s/\n---/ /g')
+          VERSION_RELEASE_NOTES=$(awk -v ver="[${{ steps.set-version-number.outputs.version_number }}]($GITHUB_SERVER_URL/$GITHUB_REPOSITORY/releases/tag/${{ env.TAG_NAME }})" '/^## / { if (p) { exit }; if ($2 == ver) { p=1; next} } p && NF' CHANGELOG.md | sed ':a;N;$!ba;s/\n---/ /g')
           echo "$VERSION_RELEASE_NOTES" >> CHANGELOG.txt
 
-      - name: Create release ${{ env.VERSION_NUMBER }}
+      - name: Create release
         uses: softprops/action-gh-release@v2
         with:
           body_path: CHANGELOG.txt
-          name: ${{ env.VERSION_NUMBER }}
-          tag_name: ${{ github.event.inputs.tag_name }}
-          draft: ${{ github.event.inputs.draft }}
-          prerelease: ${{ github.event.inputs.prerelease }}
+          name: ${{ steps.set-version-number.outputs.version_number }}
+          tag_name: ${{ env.TAG_NAME }}
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,15 @@ As far as possible, we try to adhere to [Symfony guidelines](https://symfony.com
 
 ---
 
+## [3.3.1](https://github.com/crowdsecurity/php-lapi-client/releases/tag/v3.3.1) - 2024-10-11
+[_Compare with previous release_](https://github.com/crowdsecurity/php-lapi-client/compare/v3.3.0...v3.3.1)
+
+### Fixed
+
+- Remove sensitive data from logs
+
+---
+
 ## [3.3.0](https://github.com/crowdsecurity/php-lapi-client/releases/tag/v3.3.0) - 2024-10-04
 [_Compare with previous release_](https://github.com/crowdsecurity/php-lapi-client/compare/v3.2.0...v3.3.0)
 

diff --git a/docs/DEVELOPER.md b/docs/DEVELOPER.md
@@ -132,8 +132,7 @@ Finally, run
 In order to launch integration tests, we have to set some environment variables:
 
 ```bash
-ddev exec BOUNCER_KEY=<BOUNCER_KEY> AGENT_TLS_PATH=/var/www/html/cfssl APPSEC_URL=http://crowdsec:7422 
-LAPI_URL=https://crowdsec:8080 php ./my-code/lapi-client/vendor/bin/phpunit  ./my-code/lapi-client/tests/Integration --testdox --exclude-group timeout     
+ddev exec BOUNCER_KEY=<BOUNCER_KEY> AGENT_TLS_PATH=/var/www/html/cfssl APPSEC_URL=http://crowdsec:7422 LAPI_URL=https://crowdsec:8080 php ./my-code/lapi-client/vendor/bin/phpunit  ./my-code/lapi-client/tests/Integration --testdox --exclude-group timeout     
 ```
 
 `<BOUNCER_KEY>` should have been created and retrieved before this test by running `ddev create-bouncer`.

diff --git a/src/Bouncer.php b/src/Bouncer.php
@@ -99,6 +99,16 @@ public function getStreamDecisions(
         );
     }
 
+    private function cleanHeadersForLog(array $headers): array
+    {
+        $cleanedHeaders = $headers;
+        if (array_key_exists(Constants::HEADER_APPSEC_API_KEY, $cleanedHeaders)) {
+            $cleanedHeaders[Constants::HEADER_APPSEC_API_KEY] = '***';
+        }
+
+        return $cleanedHeaders;
+    }
+
     /**
      * Process and validate input configurations.
      */
@@ -136,7 +146,7 @@ private function manageAppSecRequest(
                 'type' => 'BOUNCER_CLIENT_APPSEC_REQUEST',
                 'method' => $method,
                 'rawBody' => $rawBody,
-                'headers' => $headers,
+                'headers' => $this->cleanHeadersForLog($headers),
             ]);
 
             return $this->requestAppSec($method, $headers, $rawBody);

diff --git a/src/Constants.php b/src/Constants.php
@@ -41,5 +41,5 @@ class Constants extends CommonConstants
     /**
      * @var string The current version of this library
      */
-    public const VERSION = 'v3.3.0';
+    public const VERSION = 'v3.3.1';
 }
diff --git a/tests/Unit/AbstractClientTest.php b/tests/Unit/AbstractClientTest.php
@@ -31,6 +31,7 @@
  *
  * @covers \CrowdSec\LapiClient\Bouncer::__construct
  * @covers \CrowdSec\LapiClient\Bouncer::configure
+ * @covers \CrowdSec\LapiClient\Bouncer::cleanHeadersForLog
  */
 final class AbstractClientTest extends AbstractClient
 {
@@ -88,6 +89,29 @@ public function testClientInit()
     public function testPrivateOrProtectedMethods()
     {
         $client = new Bouncer($this->configs);
+        $headers = ['test' => 'test'];
+        $cleanedHeaders = PHPUnitUtil::callMethod(
+            $client,
+            'cleanHeadersForLog',
+            [$headers]
+        );
+        $this->assertEquals(
+            $headers,
+            $cleanedHeaders,
+            'Headers should be untouched as they are not sensitive'
+        );
+
+        $headers = ['test' => 'test', 'X-Crowdsec-Appsec-Api-Key' => '28'];
+        $cleanedHeaders = PHPUnitUtil::callMethod(
+            $client,
+            'cleanHeadersForLog',
+            [$headers]
+        );
+        $this->assertEquals(
+            ['test' => 'test', 'X-Crowdsec-Appsec-Api-Key' => '***'],
+            $cleanedHeaders,
+            'Headers should be cleaned as they are not sensitive'
+        );
 
         $fullUrl = PHPUnitUtil::callMethod(
             $client,

diff --git a/tests/Unit/BouncerTest.php b/tests/Unit/BouncerTest.php
@@ -36,6 +36,7 @@
  * @covers \CrowdSec\LapiClient\Configuration::addConnectionNodes
  * @covers \CrowdSec\LapiClient\Configuration::addAppSecNodes
  * @covers \CrowdSec\LapiClient\Configuration::validate
+ * @uses \CrowdSec\LapiClient\Bouncer::cleanHeadersForLog
  */
 final class BouncerTest extends AbstractClient
 {

diff --git a/tests/Unit/CurlTest.php b/tests/Unit/CurlTest.php
@@ -27,6 +27,7 @@
  * @uses \CrowdSec\LapiClient\Configuration::addConnectionNodes
  * @uses \CrowdSec\LapiClient\Configuration::validate
  * @uses \CrowdSec\LapiClient\Configuration::addAppSecNodes
+ * @uses \CrowdSec\LapiClient\Bouncer::cleanHeadersForLog
  *
  * @covers \CrowdSec\LapiClient\Bouncer::getStreamDecisions
  * @covers \CrowdSec\LapiClient\Bouncer::getFilteredDecisions