From 84a4bade29d27cd4aaf011f299c632303d37910a Mon Sep 17 00:00:00 2001 From: Troels Ugilt Jensen <6103205+tuj@users.noreply.github.com> Date: Fri, 12 Jun 2026 14:52:42 +0200 Subject: [PATCH 1/2] Fixed issues raised by security review --- .claude/settings.json | 1 - .github/workflows/security.yaml | 36 ++++++++++++++++++++++ README.md | 16 ++++++++-- SECURITY.md | 36 ++++++++++++++++++++++ src/Privacy/Strategy.php | 11 +++++++ src/Privacy/StrategyApplier.php | 6 +++- tests/Unit/Privacy/StrategyApplierTest.php | 20 ++++++++++-- 7 files changed, 119 insertions(+), 7 deletions(-) create mode 100644 .github/workflows/security.yaml create mode 100644 SECURITY.md diff --git a/.claude/settings.json b/.claude/settings.json index 56f9a65..0726c2b 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -1,7 +1,6 @@ { "permissions": { "allow": [ - "WebFetch(domain:raw.githubusercontent.com)", "Bash(task *)" ] } diff --git a/.github/workflows/security.yaml b/.github/workflows/security.yaml new file mode 100644 index 0000000..a5d2250 --- /dev/null +++ b/.github/workflows/security.yaml @@ -0,0 +1,36 @@ +name: Security + +env: + COMPOSE_USER: runner + +on: + pull_request: + paths: + - "composer.json" + - "docker-compose.yml" + - ".github/workflows/security.yaml" + push: + branches: + - main + - develop + paths: + - "composer.json" + - "docker-compose.yml" + - ".github/workflows/security.yaml" + schedule: + # Weekly run picks up newly-published advisories against unchanged deps. + - cron: "17 6 * * 1" + +jobs: + composer-audit: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + - name: Create docker network + run: | + docker network create frontend + + - run: | + docker compose run --rm phpfpm composer install + docker compose run --rm phpfpm composer audit diff --git a/README.md b/README.md index 6d3e654..926de37 100644 --- a/README.md +++ b/README.md @@ -346,8 +346,7 @@ class User extends AbstractITKDevEntity implements AnonymizationStatusInterface ``` Each PII field gets `#[ITKDev\EntityBundle\Privacy\Attribute\Anonymize(strategy: Strategy::Redact)]`. Strategies: -`NullValue`, `Redact`, `Hash`, `Pseudonymize` (sha1 with a `kernel.secret`-derived pepper). The bundle scans these at -compile time and ships two console commands: +`NullValue`, `Redact`, `Hash`, `Pseudonymize`. The bundle scans these at compile time and ships two console commands: - `bin/console privacy:anonymize ` — right-to-erasure: scrubs PII on the subject's User row + every row that references them via a `ManyToOne(UserInterface)` association, plus rewrites the corresponding audit history. @@ -358,6 +357,19 @@ compile time and ships two console commands: The mechanism is law-neutral; the same machinery applies to GDPR, CCPA, LGPD, PIPEDA, etc. +### GDPR semantics of strategies + +Not every strategy produces anonymous data. Pick deliberately: + +- `NullValue`, `Redact`, `Hash` — **anonymization.** Output is unlinkable to the source value and to other rows + scrubbed with the same strategy (`Hash` returns a fresh `random_bytes(32)`-derived token per call). Once applied, the + resulting column is no longer personal data. +- `Pseudonymize` — **pseudonymization.** Output is a deterministic short token derived from the cleartext and + `kernel.secret`, so rows that shared a value still collide post-scrubbing. Use this when you need to preserve + referential equality (e.g. correlating activity across tables) without retaining the cleartext. Under GDPR + Recital 26 the result is **still personal data** — apply the same access controls as you would to the cleartext, and + do not export it as "anonymized." + ## Configuration Everything is optional — see the reference table below for defaults. diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..6b5864f --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,36 @@ +# Security policy + +## Supported versions + +Security fixes are issued for the latest minor release on the `main` branch. +Older versions are not maintained — upgrade to receive fixes. + +## Reporting a vulnerability + +Please **do not** open a public GitHub issue for security problems. + +Report privately via GitHub's security advisory form: + + +Include: + +- A description of the issue and its impact. +- Steps to reproduce (a minimal failing case is ideal). +- Affected versions, if known. +- Your preferred contact for follow-up. + +## Scope + +This bundle handles personally identifiable data through its anonymization +and audit features. Reports that materially affect the confidentiality or +integrity of that data are in scope, including (non-exhaustive): + +- Anonymization strategies that fail to anonymize as documented. +- Audit log entries that retain personal data after scrubbing. +- SQL filter bypasses that expose soft-deleted or archived rows. +- Dependency vulnerabilities surfaced by the project's `composer audit` job. + +Misuse by a host application (e.g. passing untrusted YAML into bundle +configuration, exposing the privacy console commands to unauthenticated +users) is out of scope; the bundle treats its configuration and CLI +invocations as trusted. diff --git a/src/Privacy/Strategy.php b/src/Privacy/Strategy.php index 29a254e..603df65 100644 --- a/src/Privacy/Strategy.php +++ b/src/Privacy/Strategy.php @@ -4,6 +4,17 @@ namespace ITKDev\EntityBundle\Privacy; +/** + * GDPR semantics of each strategy: + * + * - NullValue, Redact, Hash → anonymization. Output is unlinkable to the + * source value and to other rows scrubbed with the same strategy. Once + * applied, the resulting column is no longer personal data. + * - Pseudonymize → pseudonymization. Output is deterministic in the source + * value and kernel.secret; rows that shared a cleartext value still + * collide post-scrubbing. The result remains personal data under GDPR + * Recital 26 — treat with the same access controls as the cleartext. + */ enum Strategy: string { case NullValue = 'null'; diff --git a/src/Privacy/StrategyApplier.php b/src/Privacy/StrategyApplier.php index c891ac1..db01d91 100644 --- a/src/Privacy/StrategyApplier.php +++ b/src/Privacy/StrategyApplier.php @@ -19,7 +19,11 @@ public function apply(Strategy $strategy, mixed $value, ?string $replacement = n return match ($strategy) { Strategy::NullValue => null, Strategy::Redact => $replacement ?? '[REDACTED]', - Strategy::Hash => null === $value ? null : hash('sha256', (string) $value.$this->pepper), + // Per-call random output — unlinkable to the source value and to any other + // row anonymized with the same strategy. Pepper is intentionally unused here + // so the result cannot be recomputed from the cleartext even by an operator + // who knows kernel.secret. + Strategy::Hash => null === $value ? null : bin2hex(random_bytes(32)), Strategy::Pseudonymize => null === $value ? null : 'user_'.substr(hash('sha256', (string) $value.$this->pepper), 0, 12), diff --git a/tests/Unit/Privacy/StrategyApplierTest.php b/tests/Unit/Privacy/StrategyApplierTest.php index 5d645dd..f58f75e 100644 --- a/tests/Unit/Privacy/StrategyApplierTest.php +++ b/tests/Unit/Privacy/StrategyApplierTest.php @@ -33,9 +33,12 @@ public function testRedactWithReplacement(): void self::assertSame('[GONE]', $this->applier->apply(Strategy::Redact, 'value', '[GONE]')); } - public function testHashOfNonNullValue(): void + public function testHashOfNonNullValueIsHex(): void { - self::assertSame(hash('sha256', 'value'.'test-pepper'), $this->applier->apply(Strategy::Hash, 'value')); + $result = $this->applier->apply(Strategy::Hash, 'value'); + + self::assertIsString($result); + self::assertMatchesRegularExpression('/^[0-9a-f]{64}$/', $result); } public function testHashOfNullReturnsNull(): void @@ -43,8 +46,19 @@ public function testHashOfNullReturnsNull(): void self::assertNull($this->applier->apply(Strategy::Hash, null)); } - public function testHashIsPepperDependent(): void + public function testHashIsNonDeterministic(): void + { + $a = $this->applier->apply(Strategy::Hash, 'alice@example.com'); + $b = $this->applier->apply(Strategy::Hash, 'alice@example.com'); + + self::assertNotSame($a, $b); + } + + public function testHashIsUnlinkableToSource(): void { + // Same source value across two different appliers (different peppers) + // must still produce unrelated outputs — Hash does not depend on the + // pepper, so determinism cannot leak via shared deployment secrets. $other = new StrategyApplier('different-pepper'); self::assertNotSame( From b1b9ee51ebf8ee8f80ef833730e9afeaff52875e Mon Sep 17 00:00:00 2001 From: Troels Ugilt Jensen <6103205+tuj@users.noreply.github.com> Date: Fri, 12 Jun 2026 15:01:36 +0200 Subject: [PATCH 2/2] Added --no-deps to audit workflow --- .github/workflows/security.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/security.yaml b/.github/workflows/security.yaml index a5d2250..e40ce4b 100644 --- a/.github/workflows/security.yaml +++ b/.github/workflows/security.yaml @@ -32,5 +32,5 @@ jobs: docker network create frontend - run: | - docker compose run --rm phpfpm composer install - docker compose run --rm phpfpm composer audit + docker compose run --rm --no-deps phpfpm composer install + docker compose run --rm --no-deps phpfpm composer audit