diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 0000000..6e7754e --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1,72 @@ +# This is a comment. +# Each line is a file pattern followed by one or more owners. + +# These owners will be the default owners for everything in +# the repo. Unless a later match takes precedence, +# @global-owner1 and @global-owner2 will be requested for +# review when someone opens a pull request. + +# Default owner for everything in the repo are members of the reviewers team. + +* @LycheeOrg/reviewers + +# Comments bellow are kept for reference + +# Order is important; the last matching pattern takes the most +# precedence. When someone opens a pull request that only +# modifies JS files, only @js-owner and not the global +# owner(s) will be requested for a review. +# *.js @js-owner #This is an inline comment. + +# You can also use email addresses if you prefer. They'll be +# used to look up users just like we do for commit author +# emails. +# *.go docs@example.com + +# Teams can be specified as code owners as well. Teams should +# be identified in the format @org/team-name. Teams must have +# explicit write access to the repository. In this example, +# the octocats team in the octo-org organization owns all .txt files. +# *.txt @octo-org/octocats + +# In this example, @doctocat owns any files in the build/logs +# directory at the root of the repository and any of its +# subdirectories. +# /build/logs/ @doctocat + +# The `docs/*` pattern will match files like +# `docs/getting-started.md` but not further nested files like +# `docs/build-app/troubleshooting.md`. +# docs/* docs@example.com + +# In this example, @octocat owns any file in an apps directory +# anywhere in your repository. +# apps/ @octocat + +# In this example, @doctocat owns any file in the `/docs` +# directory in the root of your repository and any of its +# subdirectories. +# /docs/ @doctocat + +# In this example, any change inside the `/scripts` directory +# will require approval from @doctocat or @octocat. +# /scripts/ @doctocat @octocat + +# In this example, @octocat owns any file in a `/logs` directory such as +# `/build/logs`, `/scripts/logs`, and `/deeply/nested/logs`. Any changes +# in a `/logs` directory will require approval from @octocat. +# **/logs @octocat + +# In this example, @octocat owns any file in the `/apps` +# directory in the root of your repository except for the `/apps/github` +# subdirectory, as its owners are left empty. Without an owner, changes +# to `apps/github` can be made with the approval of any user who has +# write access to the repository. +# /apps/ @octocat +# /apps/github + +# In this example, @octocat owns any file in the `/apps` +# directory in the root of your repository except for the `/apps/github` +# subdirectory, as this subdirectory has its own owner @doctocat +# /apps/ @octocat +# /apps/github @doctocat \ No newline at end of file diff --git a/.github/FUNDING.yml b/.github/FUNDING.yml new file mode 100644 index 0000000..7457bd3 --- /dev/null +++ b/.github/FUNDING.yml @@ -0,0 +1,13 @@ +# These are supported funding model platforms + +github: LycheeOrg # Replace with up to 4 GitHub Sponsors-enabled usernames e.g., [user1, user2] +# patreon: # Replace with a single Patreon username +open_collective: LycheeOrg +# ko_fi: # Replace with a single Ko-fi username +# tidelift: # Replace with a single Tidelift platform-name/package-name e.g., npm/babel +# community_bridge: # Replace with a single Community Bridge project-name e.g., cloud-foundry +# liberapay: # Replace with a single Liberapay username +# issuehunt: # Replace with a single IssueHunt username +# otechie: # Replace with a single Otechie username +# lfx_crowdfunding: # Replace with a single LFX Crowdfunding project-name e.g., cloud-foundry +# custom: # Replace with up to 4 custom sponsorship URLs e.g., ['link1', 'link2'] \ No newline at end of file diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..11561fa --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,20 @@ +version: 2 +updates: + - package-ecosystem: github-actions + directory: / + schedule: + interval: weekly + groups: + actions-deps: + patterns: + - "*" # Include all GitHub Actions + + - package-ecosystem: composer + directory: / + schedule: + interval: weekly + groups: + production-dependencies: + dependency-type: "production" + development-dependencies: + dependency-type: "development" \ No newline at end of file diff --git a/.github/release.yml b/.github/release.yml new file mode 100644 index 0000000..26823f2 --- /dev/null +++ b/.github/release.yml @@ -0,0 +1,17 @@ +changelog: + exclude: + labels: + - ignore-for-release + authors: + - octocat + - dependabot + categories: + - title: 🏕 Features + labels: + - '*' + exclude: + labels: + - dependencies + - title: 👒 Dependencies + labels: + - dependencies \ No newline at end of file diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 82158cf..11c4028 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,22 +6,36 @@ on: pull_request: branches: [ main ] +# Declare default permissions as read only. +permissions: read-all + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: ${{ !contains(github.ref, 'main') && !startsWith(github.ref, 'refs/tags/') }} # Only cancel previous runs on non-main/non-tag branches. + jobs: test: name: Tests (PHP ${{ matrix.php-version }}, PHPStan ${{ matrix.phpstan-version }}) runs-on: ubuntu-latest strategy: - fail-fast: false + fail-fast: true matrix: - php-version: [8.2, 8.3] - phpstan-version: ['2.0.*', '^2.1'] + php-version: [8.2, 8.3, 8.4, 8.5] + phpstan-version: ['2.1.3', '^2.2'] steps: - - uses: actions/checkout@v4 + - name: Harden Runner + uses: step-security/harden-runner@9af89fc71515a100421586dfdb3dc9c984fbf411 # v2.19.4 + with: + egress-policy: audit + + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false - name: Setup PHP - uses: shivammathur/setup-php@v2 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: php-version: ${{ matrix.php-version }} extensions: dom, curl, libxml, mbstring, zip @@ -29,7 +43,7 @@ jobs: - name: Cache Composer packages id: composer-cache - uses: actions/cache@v3 + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: vendor key: ${{ runner.os }}-php-${{ hashFiles('**/composer.lock') }} @@ -50,18 +64,25 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - name: Harden Runner + uses: step-security/harden-runner@9af89fc71515a100421586dfdb3dc9c984fbf411 # v2.19.4 + with: + egress-policy: audit + + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false - name: Setup PHP - uses: shivammathur/setup-php@v2 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: - php-version: 8.3 + php-version: 8.4 extensions: dom, curl, libxml, mbstring, zip coverage: none - name: Cache Composer packages id: composer-cache - uses: actions/cache@v3 + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: vendor key: ${{ runner.os }}-php-${{ hashFiles('**/composer.lock') }} @@ -79,18 +100,25 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - name: Harden Runner + uses: step-security/harden-runner@9af89fc71515a100421586dfdb3dc9c984fbf411 # v2.19.4 + with: + egress-policy: audit + + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false - name: Setup PHP - uses: shivammathur/setup-php@v2 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: - php-version: 8.3 + php-version: 8.4 extensions: dom, curl, libxml, mbstring, zip coverage: none - name: Cache Composer packages id: composer-cache - uses: actions/cache@v3 + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: vendor key: ${{ runner.os }}-php-${{ hashFiles('**/composer.lock') }} diff --git a/.github/workflows/dependency-review.yml b/.github/workflows/dependency-review.yml new file mode 100644 index 0000000..e3e8793 --- /dev/null +++ b/.github/workflows/dependency-review.yml @@ -0,0 +1,34 @@ +# Dependency Review Action +# +# This Action will scan dependency manifest files that change as part of a Pull Request, +# surfacing known-vulnerable versions of the packages declared or updated in the PR. +# Once installed, if the workflow run is marked as required, +# PRs introducing known-vulnerable packages will be blocked from merging. +# +# Source repository: https://github.com/actions/dependency-review-action +name: 'Dependency Review' +on: [pull_request] + +permissions: + contents: read + +jobs: + dependency-review: + runs-on: ubuntu-latest + steps: + - name: Harden Runner + uses: step-security/harden-runner@9af89fc71515a100421586dfdb3dc9c984fbf411 # v2.19.4 + with: + egress-policy: audit + + - name: 'Checkout Repository' + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + - name: 'Dependency Review' + uses: actions/dependency-review-action@a1d282b36b6f3519aa1f3fc636f609c47dddb294 # v5.0.0 + # with: + # No fix available yet + # Note that the model is directly baked into the inage + # So the risk is limited. + # allow-ghsas: GHSA-hqmj-h5c6-369m \ No newline at end of file diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml new file mode 100644 index 0000000..d6d8397 --- /dev/null +++ b/.github/workflows/scorecard.yml @@ -0,0 +1,77 @@ +# This workflow uses actions that are not certified by GitHub. They are provided +# by a third-party and are governed by separate terms of service, privacy +# policy, and support documentation. + +name: Scorecard supply-chain security +on: + # For Branch-Protection check. Only the default branch is supported. See + # https://github.com/ossf/scorecard/blob/main/docs/checks.md#branch-protection + branch_protection_rule: + # To guarantee Maintained check is occasionally updated. See + # https://github.com/ossf/scorecard/blob/main/docs/checks.md#maintained + schedule: + - cron: '00 02 * * 1' + push: + branches: [ "main" ] + +# Declare default permissions as read only. +permissions: read-all + +jobs: + analysis: + name: Scorecard analysis + runs-on: ubuntu-latest + permissions: + # Needed to upload the results to code-scanning dashboard. + security-events: write + # Needed to publish results and get a badge (see publish_results below). + id-token: write + # Uncomment the permissions below if installing in a private repository. + # contents: read + # actions: read + + steps: + - name: Harden Runner + uses: step-security/harden-runner@9af89fc71515a100421586dfdb3dc9c984fbf411 # v2.19.4 + with: + egress-policy: audit + + - name: "Checkout code" + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + + - name: "Run analysis" + uses: ossf/scorecard-action@4eaacf0543bb3f2c246792bd56e8cdeffafb205a # v2.4.3 + with: + results_file: results.sarif + results_format: sarif + # (Optional) "write" PAT token. Uncomment the `repo_token` line below if: + # - you want to enable the Branch-Protection check on a *public* repository, or + # - you are installing Scorecard on a *private* repository + # To create the PAT, follow the steps in https://github.com/ossf/scorecard-action#authentication-with-pat. + # repo_token: ${{ secrets.SCORECARD_TOKEN }} + + # Public repositories: + # - Publish results to OpenSSF REST API for easy access by consumers + # - Allows the repository to include the Scorecard badge. + # - See https://github.com/ossf/scorecard-action#publishing-results. + # For private repositories: + # - `publish_results` will always be set to `false`, regardless + # of the value entered here. + publish_results: true + + # Upload the results as artifacts (optional). Commenting out will disable uploads of run results in SARIF + # format to the repository Actions tab. + - name: "Upload artifact" + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: SARIF file + path: results.sarif + retention-days: 5 + + # Upload the results to GitHub's code scanning dashboard. + - name: "Upload to code-scanning" + uses: github/codeql-action/upload-sarif@08bc0cf022445eacafaa248bf48da20f26b8fd40 # v2.16.4 + with: + sarif_file: results.sarif \ No newline at end of file diff --git a/LICENSE b/LICENSE index 27fc6cf..52f334b 100644 --- a/LICENSE +++ b/LICENSE @@ -1,6 +1,7 @@ MIT License Copyright (c) 2025 BuiltFast.com +Copyright (c) 2026 LycheeOrg Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..32ac659 --- /dev/null +++ b/Makefile @@ -0,0 +1,41 @@ +.PHONY: help install update test test-coverage pint pint-test stan check ci clean + +help: + @echo "Available targets:" + @echo " install Install composer dependencies" + @echo " update Update composer dependencies" + @echo " test Run the test suite (Pest)" + @echo " test-coverage Run the test suite with coverage report" + @echo " pint Fix code style (Laravel Pint)" + @echo " pint-test Check code style without fixing" + @echo " stan Run static analysis (PHPStan)" + @echo " check Run pint-test and stan (no tests)" + @echo " ci Run test, pint-test and stan (mirrors CI)" + @echo " clean Remove vendor and lock file" + +install: + composer install + +update: + composer update + +test: + vendor/bin/pest +test-coverage: + vendor/bin/pest --coverage + +pint: + vendor/bin/pint + +pint-test: + vendor/bin/pint --test + +stan: + vendor/bin/phpstan analyze + +check: pint-test stan + +ci: test pint-test stan + +clean: + rm -rf vendor composer.lock diff --git a/README.md b/README.md index 37ff148..050e77d 100644 --- a/README.md +++ b/README.md @@ -1,9 +1,10 @@ # PHPStan SensitiveParameter Detector -[![CI](https://github.com/built-fast/phpstan-sensitive-parameter/workflows/CI/badge.svg)](https://github.com/built-fast/phpstan-sensitive-parameter/actions) -[![Latest Stable Version](https://poser.pugx.org/built-fast/phpstan-sensitive-parameter/v/stable)](https://packagist.org/packages/built-fast/phpstan-sensitive-parameter) -[![Total Downloads](https://poser.pugx.org/built-fast/phpstan-sensitive-parameter/downloads)](https://packagist.org/packages/built-fast/phpstan-sensitive-parameter) -[![License](https://poser.pugx.org/built-fast/phpstan-sensitive-parameter/license)](https://packagist.org/packages/built-fast/phpstan-sensitive-parameter) +[![CI](https://github.com/LycheeOrg/phpstan-sensitive-parameter-values/workflows/CI/badge.svg)](https://github.com/LycheeOrg/phpstan-sensitive-parameter-values/actions) + + +[![License](https://poser.pugx.org/LycheeOrg/phpstan-sensitive-parameter-values/license)](https://packagist.org/packages/LycheeOrg/phpstan-sensitive-parameter-values) +[![OpenSSF Scorecard][ossf-shield]](https://securityscorecards.dev/viewer/?uri=github.com/LycheeOrg/phpstan-sensitive-parameter-values) A PHPStan extension that detects parameters that might contain sensitive information and should be marked with the `#[\SensitiveParameter]` attribute (added in PHP 8.2+). @@ -16,12 +17,13 @@ Learn more: [PHP RFC: Redact parameters in back traces](https://wiki.php.net/rfc ## Requirements - PHP 8.2 or higher -- PHPStan 2.0 or higher +- PHPStan 2.1.3 or higher (`SensitiveParameterPropagationRule` relies on + `getAttributes()` reflection support added in 2.1.3) ## Installation ```bash -composer require --dev built-fast/phpstan-sensitive-parameter +composer require --dev lychee-org/phpstan-sensitive-parameter-values ``` ## Usage @@ -32,9 +34,139 @@ Alternatively, include the extension in your PHPStan configuration: ```neon includes: - - vendor/built-fast/phpstan-sensitive-parameter/extension.neon + - vendor/lychee-org/phpstan-sensitive-parameter-values/extension.neon +``` + +## Typed `SensitiveParameterValue` + +PHP's built-in `\SensitiveParameterValue::getValue()` is natively typed as +`mixed`, so calling it normally loses type information. This extension ships +a PHPStan stub that declares `SensitiveParameterValue` as generic over the +type of the value passed to its constructor, so PHPStan can narrow the +return type of `getValue()` accordingly: + +```php +function example(string $password): void { + $sensitive = new \SensitiveParameterValue($password); + + // PHPStan now sees $sensitive as SensitiveParameterValue + // and infers the return type of getValue() as string, not mixed. + $plain = $sensitive->getValue(); +} ``` +This is most useful when inspecting exception traces, where PHP replaces +sensitive arguments with `SensitiveParameterValue` instances: + +```php +foreach ($exception->getTrace() as $frame) { + foreach ($frame['args'] ?? [] as $arg) { + if ($arg instanceof \SensitiveParameterValue) { + // getValue() keeps the original argument's type. + $original = $arg->getValue(); + } + } +} +``` + +## Propagating sensitivity through the call graph + +Marking a parameter `#[\SensitiveParameter]` only protects that one call +frame. If the value is then forwarded unchanged into a callee whose +corresponding parameter is *not* marked sensitive, protection stops there: an +exception thrown from inside the callee will still expose the value in +plaintext. + +```php +class AuthService { + // $password is marked sensitive here... + public function authenticate(#[\SensitiveParameter] string $password): bool { + // ...but login()'s parameter isn't, so the value is unprotected + // as soon as it enters login()'s stack frame. + return $this->login($password); + } + + public function login(string $password): bool { + // ... + } +} +``` + +`SensitiveParameterPropagationRule` flags `login()`'s `$password` +in this example, with: + +``` +Parameter $password is marked #[\SensitiveParameter] but is passed to a +parameter ($password) that is not itself marked with #[\SensitiveParameter]. +Add the attribute there too or ignore with +`@phpstan-ignore sensitiveParameter.propagation`. +``` + +This is detected across method calls, static calls, constructors, and plain +function calls. Only simple, unmodified pass-through arguments (a bare +`$variable` matching a sensitive parameter of the enclosing function/method) +are tracked — values that are transformed, wrapped, or reassigned before +being passed on are not. + +## Storing sensitive values safely + +Marking a parameter sensitive prevents it from leaking through stack traces, +but that protection is undone if the raw value is then saved into a property +— anything that inspects, dumps, or serializes the object exposes it again. +`SensitiveParameterStorageRule` requires sensitive values to be wrapped in +`\SensitiveParameterValue` before being stored: + +```php +class Credentials { + private string $password; // ❌ raw storage + + public function __construct(#[\SensitiveParameter] string $password) { + $this->password = $password; // flagged: sensitiveParameter.unwrappedStorage + } +} +``` + +```php +class Credentials { + private \SensitiveParameterValue $password; // ✅ wrapped storage + + public function __construct(#[\SensitiveParameter] string $password) { + $this->password = new \SensitiveParameterValue($password); + } +} +``` + +Constructor property promotion is also checked, since promotion assigns the +raw value directly with no place to wrap it: + +```php +class Credentials { + public function __construct( + // flagged: sensitiveParameter.unwrappedPromotion + #[\SensitiveParameter] private readonly string $password, + ) {} +} +``` + +A value that's already wrapped is also checked: unwrapping it via +`->getValue()` right before storing defeats the point of wrapping it in the +first place, so it's flagged too: + +```php +class Credentials { + private string $password; + + public function __construct(\SensitiveParameterValue $password) { + // flagged: sensitiveParameter.unwrappedGetValue + $this->password = $password->getValue(); + } +} +``` + +Only direct, unmodified assignments of a bare `$variable` (or a bare +`->getValue()` call on one) into a property are detected; values transformed +before being stored are not tracked. + ## What it detects The rule detects parameters with names containing common sensitive keywords: @@ -110,7 +242,7 @@ includes: services: # Override the default service with custom keywords - - class: BuiltFast\Rules\SensitiveParameterDetectorRule + class: LycheeOrg\PHPStan\Rules\SensitiveParameterDetectorRule arguments: - ['password', 'apikey', 'token', 'banking', 'medical'] # Your custom keywords tags: @@ -174,7 +306,7 @@ parameters to be more specific (e.g., `$applicationToInstall`, `$authService`, ## Reporting Issues -Found a bug or have a feature request? Please [report it on GitHub](https://github.com/built-fast/phpstan-sensitive-parameter/issues). +Found a bug or have a feature request? Please [report it on GitHub](https://github.com/LycheeOrg/phpstan-sensitive-parameter-values/issues). When reporting issues, please include: @@ -190,8 +322,8 @@ Contributions are welcome! Please feel free to submit a Pull Request. For major **Development setup:** ```bash -git clone https://github.com/built-fast/phpstan-sensitive-parameter.git -cd phpstan-sensitive-parameter +git clone https://github.com/LycheeOrg/phpstan-sensitive-parameter-values.git +cd phpstan-sensitive-parameter-values composer install ``` @@ -206,3 +338,5 @@ vendor/bin/pint --test # Code style check ## License MIT License - see [`LICENSE`](./LICENSE) for details. + +[ossf-shield]: https://api.securityscorecards.dev/projects/github.com/LycheeOrg/phpstan-sensitive-parameter-values/badge diff --git a/composer.json b/composer.json index 59cdae8..8870392 100644 --- a/composer.json +++ b/composer.json @@ -1,9 +1,9 @@ { - "name": "built-fast/phpstan-sensitive-parameter", - "description": "PHPStan extension for detecting parameters that should use SensitiveParameter", + "name": "lychee-org/phpstan-sensitive-parameter-values", + "description": "PHPStan extension for better support of SensitiveParameter and SensitiveParameterValue attributes.", "type": "phpstan-extension", "license": "MIT", - "homepage": "https://github.com/built-fast/phpstan-sensitive-parameter", + "homepage": "https://github.com/LycheeOrg/phpstan-sensitive-parameter-values", "keywords": [ "phpstan", "phpstan-extension", @@ -14,23 +14,27 @@ "code-quality" ], "authors": [ + { + "name": "LycheeOrg", + "homepage": "https://github.com/LycheeOrg/phpstan-sensitive-parameter-values" + }, { "name": "Josh Priddle", "homepage": "https://github.com/built-fast/phpstan-sensitive-parameter" } ], "support": { - "issues": "https://github.com/built-fast/phpstan-sensitive-parameter/issues", - "forum": "https://github.com/built-fast/phpstan-sensitive-parameter/discussions", - "source": "https://github.com/built-fast/phpstan-sensitive-parameter" + "issues": "https://github.com/LycheeOrg/phpstan-sensitive-parameter-values/issues", + "forum": "https://github.com/LycheeOrg/phpstan-sensitive-parameter-values/discussions", + "source": "https://github.com/LycheeOrg/phpstan-sensitive-parameter-values" }, "require": { "php": "^8.2", - "phpstan/phpstan": "^2.0" + "phpstan/phpstan": "^2.1.3" }, "autoload": { "psr-4": { - "BuiltFast\\": "src/" + "LycheeOrg\\PHPStan\\": "src/" } }, "autoload-dev": { diff --git a/extension.neon b/extension.neon index 48dafcc..8a2526d 100644 --- a/extension.neon +++ b/extension.neon @@ -1,5 +1,17 @@ +parameters: + stubFiles: + - stubs/SensitiveParameterValue.stub + services: - - class: BuiltFast\Rules\SensitiveParameterDetectorRule + class: LycheeOrg\PHPStan\Rules\SensitiveParameterDetectorRule + tags: + - phpstan.rules.rule + - + class: LycheeOrg\PHPStan\Rules\SensitiveParameterPropagationRule + tags: + - phpstan.rules.rule + - + class: LycheeOrg\PHPStan\Rules\SensitiveParameterStorageRule tags: - phpstan.rules.rule diff --git a/phpstan.neon b/phpstan.neon index 0146844..dbbfc4d 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -8,4 +8,4 @@ parameters: - tests/Fixtures/* # PHP version (match your composer.json requirement) - phpVersion: 80000 \ No newline at end of file + phpVersion: 80200 \ No newline at end of file diff --git a/phpunit.xml b/phpunit.xml index f0b8e88..044c60c 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -7,8 +7,17 @@ tests/Unit - - tests/Feature + + tests/Detector + + + tests/Types + + + tests/Propagation + + + tests/Storage diff --git a/src/Rules/SensitiveParameterDetectorRule.php b/src/Rules/SensitiveParameterDetectorRule.php index 9589d93..4e68ece 100644 --- a/src/Rules/SensitiveParameterDetectorRule.php +++ b/src/Rules/SensitiveParameterDetectorRule.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace BuiltFast\Rules; +namespace LycheeOrg\PHPStan\Rules; use PhpParser\Node; use PhpParser\Node\FunctionLike; @@ -154,6 +154,10 @@ public function processNode(Node $node, Scope $scope): array $paramName = $param->var->name; + if (str_starts_with($paramName, 'is') || $this->hasBoolType($param->type)) { + continue; + } + $paramHasSensitiveAttribute = false; if ($param->attrGroups) { @@ -196,4 +200,19 @@ public function processNode(Node $node, Scope $scope): array return $errors; } + + /** + * Determine whether a parameter's type hint is exactly bool, optionally + * nullable (`bool` or `?bool`). Union types such as `bool|string` are not + * considered bool, since the parameter can still hold a non-boolean + * value. + */ + private function hasBoolType(?Node $type): bool + { + if ($type instanceof Node\NullableType) { + $type = $type->type; + } + + return $type instanceof Node\Identifier && mb_strtolower($type->toString()) === 'bool'; + } } diff --git a/src/Rules/SensitiveParameterPropagationRule.php b/src/Rules/SensitiveParameterPropagationRule.php new file mode 100644 index 0000000..dce6465 --- /dev/null +++ b/src/Rules/SensitiveParameterPropagationRule.php @@ -0,0 +1,282 @@ + + */ +final class SensitiveParameterPropagationRule implements Rule +{ + public function __construct(private ReflectionProvider $reflectionProvider) + { + } + + public function getNodeType(): string + { + return CallLike::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if ( + ! $node instanceof New_ + && ! $node instanceof MethodCall + && ! $node instanceof NullsafeMethodCall + && ! $node instanceof StaticCall + && ! $node instanceof FuncCall + ) { + return []; + } + + $callerFunction = $scope->getFunction(); + if ($callerFunction === null) { + return []; + } + + $callerParams = $callerFunction->getParameters(); + if ($callerParams === []) { + return []; + } + + $callerFunctionProtected = $this->hasSensitiveAttribute($callerFunction->getAttributes()); + + $callee = $this->resolveCallee($node, $scope); + if ($callee === null) { + return []; + } + + [$calleeAttributes, $calleeParams] = $callee; + if ($calleeParams === []) { + return []; + } + + $calleeFunctionProtected = $this->hasSensitiveAttribute($calleeAttributes); + + $errors = []; + + foreach ($node->getArgs() as $position => $arg) { + if (! $arg->value instanceof Variable || ! is_string($arg->value->name)) { + continue; + } + + $callerParam = $this->findParameterByName($callerParams, $arg->value->name); + if ($callerParam === null) { + continue; + } + + if (! $callerFunctionProtected && ! $this->hasSensitiveAttribute($callerParam->getAttributes())) { + continue; + } + + if ($scope->getVariableType($arg->value->name)->isConstantValue()->yes()) { + // The variable has been reassigned to a literal/constant value + // and no longer certainly holds the original sensitive value. + continue; + } + + $calleeParam = $this->resolveCalleeParameter($calleeParams, $arg, $position); + if ($calleeParam === null) { + continue; + } + + if ($calleeFunctionProtected || $this->hasSensitiveAttribute($calleeParam->getAttributes())) { + continue; + } + + $errors[] = RuleErrorBuilder::message(sprintf( + 'Parameter $%s is marked #[\\SensitiveParameter] but is passed to a parameter ($%s) that is not itself marked with #[\\SensitiveParameter]. Add the attribute there too or ignore with `@phpstan-ignore sensitiveParameter.propagation`.', + $callerParam->getName(), + $calleeParam->getName(), + )) + ->identifier('sensitiveParameter.propagation') + ->build(); + } + + return $errors; + } + + /** + * @return array{0: AttributeReflection[], 1: ExtendedParameterReflection[]}|null + */ + private function resolveCallee(CallLike $node, Scope $scope): ?array + { + if ($node instanceof New_) { + $type = $scope->getType($node); + if (! $type->isObject()->yes() || ! $type->hasMethod('__construct')->yes()) { + return null; + } + + $method = $type->getMethod('__construct', $scope); + $variant = $this->selectVariant($scope, $node, $method->getVariants(), $method->getNamedArgumentsVariants()); + + return $variant === null ? null : [$method->getAttributes(), $variant->getParameters()]; + } + + if ($node instanceof MethodCall || $node instanceof NullsafeMethodCall) { + if (! $node->name instanceof Node\Identifier) { + return null; + } + + $type = $scope->getType($node->var); + if (! $type->isObject()->yes()) { + return null; + } + + $methodName = $node->name->toString(); + if (! $type->hasMethod($methodName)->yes()) { + return null; + } + + $method = $type->getMethod($methodName, $scope); + $variant = $this->selectVariant($scope, $node, $method->getVariants(), $method->getNamedArgumentsVariants()); + + return $variant === null ? null : [$method->getAttributes(), $variant->getParameters()]; + } + + if ($node instanceof StaticCall) { + if (! $node->name instanceof Node\Identifier) { + return null; + } + + $type = $node->class instanceof Node\Name + ? $scope->resolveTypeByName($node->class) + : $scope->getType($node->class); + + if (! $type->isObject()->yes()) { + return null; + } + + $methodName = $node->name->toString(); + if (! $type->hasMethod($methodName)->yes()) { + return null; + } + + $method = $type->getMethod($methodName, $scope); + $variant = $this->selectVariant($scope, $node, $method->getVariants(), $method->getNamedArgumentsVariants()); + + return $variant === null ? null : [$method->getAttributes(), $variant->getParameters()]; + } + + if (! $node instanceof FuncCall || ! $node->name instanceof Node\Name) { + return null; + } + + if (! $this->reflectionProvider->hasFunction($node->name, $scope)) { + return null; + } + + $function = $this->reflectionProvider->getFunction($node->name, $scope); + $variant = $this->selectVariant($scope, $node, $function->getVariants(), $function->getNamedArgumentsVariants()); + + return $variant === null ? null : [$function->getAttributes(), $variant->getParameters()]; + } + + /** + * Picks the variant matching the actual call-site arguments (handling + * overloads and named-argument variants) instead of blindly assuming the + * first declared variant is the right one. + * + * @param ExtendedParametersAcceptor[] $variants + * @param ExtendedParametersAcceptor[]|null $namedArgumentsVariants + */ + private function selectVariant(Scope $scope, CallLike $node, array $variants, ?array $namedArgumentsVariants): ?ExtendedParametersAcceptor + { + if ($variants === []) { + return null; + } + + $selected = ParametersAcceptorSelector::selectFromArgs($scope, $node->getArgs(), $variants, $namedArgumentsVariants); + + return $selected instanceof ExtendedParametersAcceptor ? $selected : null; + } + + /** + * @param ExtendedParameterReflection[] $params + */ + private function findParameterByName(array $params, string $name): ?ExtendedParameterReflection + { + foreach ($params as $param) { + if ($param->getName() === $name) { + return $param; + } + } + + return null; + } + + /** + * @param ExtendedParameterReflection[] $calleeParams + */ + private function resolveCalleeParameter(array $calleeParams, Arg $arg, int $position): ?ExtendedParameterReflection + { + if ($arg->name instanceof Node\Identifier) { + $named = $this->findParameterByName($calleeParams, $arg->name->toString()); + + return $named ?? $this->variadicParameter($calleeParams); + } + + return $calleeParams[$position] ?? $this->variadicParameter($calleeParams); + } + + /** + * @param ExtendedParameterReflection[] $calleeParams + */ + private function variadicParameter(array $calleeParams): ?ExtendedParameterReflection + { + $lastParam = $calleeParams[count($calleeParams) - 1] ?? null; + + return ($lastParam !== null && $lastParam->isVariadic()) ? $lastParam : null; + } + + /** + * @param AttributeReflection[] $attributes + */ + private function hasSensitiveAttribute(array $attributes): bool + { + foreach ($attributes as $attribute) { + $name = $attribute->getName(); + if ($name === 'SensitiveParameter' || $name === '\\SensitiveParameter') { + return true; + } + } + + return false; + } +} diff --git a/src/Rules/SensitiveParameterStorageRule.php b/src/Rules/SensitiveParameterStorageRule.php new file mode 100644 index 0000000..22b944d --- /dev/null +++ b/src/Rules/SensitiveParameterStorageRule.php @@ -0,0 +1,275 @@ +password = $password;` instead of + * `$this->password = new \SensitiveParameterValue($password);` + * 2. A sensitive parameter is promoted directly into a property + * (constructor property promotion), which performs the raw assignment + * implicitly and offers no place to wrap the value. + * 3. A parameter already typed as \SensitiveParameterValue is unwrapped via + * `->getValue()` before being stored: `$this->password = + * $password->getValue();` instead of storing the wrapped value itself. + * + * Only direct, unmodified assignments of a bare `$variable` (or a bare + * `->getValue()` call on one) are detected. Values that are transformed + * before being stored are not tracked. If a parameter's variable is + * reassigned anywhere in the function, it is dropped from tracking entirely + * (for all of its uses) since it can no longer be proven to still hold the + * original sensitive value. + * + * @implements Rule + */ +final class SensitiveParameterStorageRule implements Rule +{ + public function getNodeType(): string + { + return FunctionLike::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (! $node instanceof ClassMethod && ! $node instanceof Function_) { + return []; + } + + $functionProtected = $this->hasSensitiveAttributeGroups($node->attrGroups); + + /** @var array $sensitiveParamNames */ + $sensitiveParamNames = []; + /** @var array $wrappedParamNames */ + $wrappedParamNames = []; + $errors = []; + + foreach ($node->getParams() as $param) { + if (! $param->var instanceof Variable || ! is_string($param->var->name)) { + continue; + } + + if ($this->isSensitiveParameterValueType($param->type)) { + $wrappedParamNames[$param->var->name] = true; + } + + $paramSensitive = $functionProtected || $this->hasSensitiveAttributeGroups($param->attrGroups); + if (! $paramSensitive) { + continue; + } + + $sensitiveParamNames[$param->var->name] = true; + + if ($param->flags !== 0 && ! $this->isSensitiveParameterValueType($param->type)) { + $errors[] = RuleErrorBuilder::message(sprintf( + 'Promoted property $%s receives a sensitive value but assigns it directly, bypassing \\SensitiveParameterValue. Declare the property without promotion and assign `new \\SensitiveParameterValue($%s)` in the constructor body, or ignore with `@phpstan-ignore sensitiveParameter.unwrappedPromotion`.', + $param->var->name, + $param->var->name, + )) + ->identifier('sensitiveParameter.unwrappedPromotion') + ->line($param->getStartLine()) + ->build(); + } + } + + if (($sensitiveParamNames === [] && $wrappedParamNames === []) || $node->getStmts() === null) { + return $errors; + } + + $assigns = $this->findAssignsInCurrentScope($node->getStmts()); + + // If a parameter's variable is reassigned anywhere in the function, + // it can no longer be proven to still hold the original sensitive + // value at any of its uses, so stop treating it as sensitive. + foreach ($assigns as $assign) { + if ($assign->var instanceof Variable && is_string($assign->var->name)) { + unset($sensitiveParamNames[$assign->var->name], $wrappedParamNames[$assign->var->name]); + } + } + + foreach ($assigns as $assign) { + foreach ($this->checkAssign($assign, $sensitiveParamNames, $wrappedParamNames) as $error) { + $errors[] = $error; + } + } + + return $errors; + } + + /** + * Finds Assign nodes in the given statements, without descending into + * nested function-like scopes (closures, arrow functions, nested + * methods/functions), which have their own independent parameter scope + * and may shadow the outer sensitive parameter names. + * + * @param Node\Stmt[] $stmts + * @return Assign[] + */ + private function findAssignsInCurrentScope(array $stmts): array + { + $visitor = new class extends NodeVisitorAbstract { + /** @var Assign[] */ + public array $assigns = []; + + /** + * @return int|null + */ + public function enterNode(Node $node) + { + if ($node instanceof FunctionLike) { + return NodeVisitor::DONT_TRAVERSE_CHILDREN; + } + + if ($node instanceof Assign) { + $this->assigns[] = $node; + } + + return null; + } + }; + + $traverser = new NodeTraverser(); + $traverser->addVisitor($visitor); + $traverser->traverse($stmts); + + return $visitor->assigns; + } + + /** + * @param array $sensitiveParamNames + * @param array $wrappedParamNames + * @return IdentifierRuleError[] + */ + private function checkAssign(Assign $assign, array $sensitiveParamNames, array $wrappedParamNames): array + { + if (! $assign->var instanceof PropertyFetch && ! $assign->var instanceof StaticPropertyFetch) { + return []; + } + + if ( + $assign->expr instanceof Variable + && is_string($assign->expr->name) + && isset($sensitiveParamNames[$assign->expr->name]) + ) { + return [ + RuleErrorBuilder::message(sprintf( + '$%s is a sensitive value and must be wrapped in \\SensitiveParameterValue before being stored, e.g. `... = new \\SensitiveParameterValue($%s);`. Ignore with `@phpstan-ignore sensitiveParameter.unwrappedStorage`.', + $assign->expr->name, + $assign->expr->name, + )) + ->identifier('sensitiveParameter.unwrappedStorage') + ->line($assign->getStartLine()) + ->build(), + ]; + } + + if ( + $assign->expr instanceof MethodCall + && $assign->expr->name instanceof Node\Identifier + && $assign->expr->name->toString() === 'getValue' + && $assign->expr->var instanceof Variable + && is_string($assign->expr->var->name) + && isset($wrappedParamNames[$assign->expr->var->name]) + ) { + return [ + RuleErrorBuilder::message(sprintf( + '$%s->getValue() unwraps a \\SensitiveParameterValue and must not be stored raw. Store $%s itself instead of calling getValue() on it. Ignore with `@phpstan-ignore sensitiveParameter.unwrappedGetValue`.', + $assign->expr->var->name, + $assign->expr->var->name, + )) + ->identifier('sensitiveParameter.unwrappedGetValue') + ->line($assign->getStartLine()) + ->build(), + ]; + } + + return []; + } + + /** + * @param Node\AttributeGroup[] $attrGroups + */ + private function hasSensitiveAttributeGroups(array $attrGroups): bool + { + foreach ($attrGroups as $attrGroup) { + foreach ($attrGroup->attrs as $attr) { + $attrName = $attr->name->toString(); + if ($attrName === 'SensitiveParameter' || $attrName === '\SensitiveParameter') { + return true; + } + } + } + + return false; + } + + /** + * True if every non-null member of the type is \SensitiveParameterValue. + * A plain nullable wrapper (`?SensitiveParameterValue` or + * `SensitiveParameterValue|null`) is safe, but a mixed union like + * `SensitiveParameterValue|string` is not: the raw string branch could + * still be stored unwrapped. + */ + private function isSensitiveParameterValueType(?Node $type): bool + { + if ($type instanceof Node\NullableType) { + $type = $type->type; + } + + if ($type instanceof Node\UnionType) { + foreach ($type->types as $subType) { + if ($this->isNullType($subType)) { + continue; + } + + if (! $this->isSensitiveParameterValueType($subType)) { + return false; + } + } + + return true; + } + + if ($type instanceof Node\Name) { + $name = $type->toString(); + + return $name === 'SensitiveParameterValue' || $name === '\\SensitiveParameterValue'; + } + + return false; + } + + private function isNullType(Node $type): bool + { + return $type instanceof Node\Identifier && mb_strtolower($type->toString()) === 'null'; + } +} diff --git a/stubs/SensitiveParameterValue.stub b/stubs/SensitiveParameterValue.stub new file mode 100644 index 0000000..e501920 --- /dev/null +++ b/stubs/SensitiveParameterValue.stub @@ -0,0 +1,30 @@ +rule = new SensitiveParameterDetectorRule(); +}); + +it('detects basic sensitive parameters', function () { + $this->analyse([__DIR__.'/../Fixtures/Detector/BasicSensitiveParameters.php'], [ + [ + 'Parameter $userSecret in Tests\\Fixtures\\Detector\\BasicSensitiveParameters::regularFunction might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 13, + ], + [ + 'Parameter $credential in Tests\\Fixtures\\Detector\\BasicSensitiveParameters::regularFunction might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 13, + ], + [ + 'Parameter $password in Tests\\Fixtures\\Detector\\BasicSensitiveParameters::authenticate might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 19, + ], + [ + 'Parameter $apikey in Tests\\Fixtures\\Detector\\BasicSensitiveParameters::setApiCredentials might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 24, + ], + [ + 'Parameter $apisecret in Tests\\Fixtures\\Detector\\BasicSensitiveParameters::setApiCredentials might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 24, + ], + [ + 'Parameter $cardNumber in Tests\\Fixtures\\Detector\\BasicSensitiveParameters::processPayment might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 29, + ], + [ + 'Parameter $cvv in Tests\\Fixtures\\Detector\\BasicSensitiveParameters::processPayment might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 29, + ], + [ + 'Parameter $creditCardToken in Tests\\Fixtures\\Detector\\BasicSensitiveParameters::processPayment might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 29, + ], + [ + 'Parameter $ssn in Tests\\Fixtures\\Detector\\BasicSensitiveParameters::storeUserData might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 34, + ], + [ + 'Parameter $privateKey in Tests\\Fixtures\\Detector\\BasicSensitiveParameters::storeUserData might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 34, + ], + [ + 'Parameter $authToken in Tests\\Fixtures\\Detector\\BasicSensitiveParameters::handleTokens might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 39, + ], + [ + 'Parameter $refreshToken in Tests\\Fixtures\\Detector\\BasicSensitiveParameters::handleTokens might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 39, + ], + [ + 'Parameter $accessToken in Tests\\Fixtures\\Detector\\BasicSensitiveParameters::handleTokens might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 39, + ], + [ + 'Parameter $password in globalAuthFunction might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 46, + ], + ]); +}); + +test('does not warn for protected sensitive parameters', function () { + $this->analyse([__DIR__.'/../Fixtures/Detector/ProtectedSensitiveParameters.php'], [ + [ + 'Parameter $apikey in Tests\\Fixtures\\Detector\\ProtectedSensitiveParameters::setApiCredentials might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 22, + ], + [ + 'Parameter $cvv in Tests\\Fixtures\\Detector\\ProtectedSensitiveParameters::processPayment might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 28, + ], + ]); +}); + +test('detects edge cases correctly', function () { + $this->analyse([__DIR__.'/../Fixtures/Detector/EdgeCases.php'], [ + [ + 'Parameter $apikey in Tests\\Fixtures\\Detector\\EdgeCases::__construct might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 13, + ], + [ + 'Parameter $credential in Tests\\Fixtures\\Detector\\EdgeCases::staticMethod might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 18, + ], + [ + 'Parameter $userPassword in Tests\\Fixtures\\Detector\\EdgeCases::partialMatches might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 24, + ], + [ + 'Parameter $secretKey in Tests\\Fixtures\\Detector\\EdgeCases::partialMatches might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 24, + ], + [ + 'Parameter $apiToken in Tests\\Fixtures\\Detector\\EdgeCases::partialMatches might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 24, + ], + [ + 'Parameter $Password in Tests\\Fixtures\\Detector\\EdgeCases::caseVariations might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 30, + ], + [ + 'Parameter $SECRET in Tests\\Fixtures\\Detector\\EdgeCases::caseVariations might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 30, + ], + [ + 'Parameter $Token in Tests\\Fixtures\\Detector\\EdgeCases::caseVariations might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 30, + ], + [ + 'Parameter $myPassword in Tests\\Fixtures\\Detector\\EdgeCases::mixedCaseCompounds might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 36, + ], + [ + 'Parameter $userSecret in Tests\\Fixtures\\Detector\\EdgeCases::mixedCaseCompounds might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 36, + ], + [ + 'Parameter $appToken in Tests\\Fixtures\\Detector\\EdgeCases::mixedCaseCompounds might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 36, + ], + [ + 'Parameter $passwordless in Tests\\Fixtures\\Detector\\EdgeCases::falsePositives might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 42, + ], + [ + 'Parameter $secretion in Tests\\Fixtures\\Detector\\EdgeCases::falsePositives might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 42, + ], + [ + 'Parameter $password in Tests\\Fixtures\\Detector\\EdgeCases::multiplePasswords might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 49, + ], + [ + 'Parameter $confirmPassword in Tests\\Fixtures\\Detector\\EdgeCases::multiplePasswords might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 49, + ], + [ + 'Parameter $oldPassword in Tests\\Fixtures\\Detector\\EdgeCases::multiplePasswords might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 49, + ], + [ + 'Parameter $password123 in Tests\\Fixtures\\Detector\\EdgeCases::unusualCases might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 55, + ], + [ + 'Parameter $password_hash in Tests\\Fixtures\\Detector\\EdgeCases::unusualCases might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 55, + ], + [ + 'Parameter $secret in Tests\\Fixtures\\Detector\\EdgeCases::privateMethod might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 61, + ], + [ + 'Parameter $token in Tests\\Fixtures\\Detector\\EdgeCases::protectedMethod might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 66, + ], + ]); +}); diff --git a/tests/Detector/BooleanCasesTest.php b/tests/Detector/BooleanCasesTest.php new file mode 100644 index 0000000..422e23a --- /dev/null +++ b/tests/Detector/BooleanCasesTest.php @@ -0,0 +1,20 @@ +analyse([__DIR__.'/../Fixtures/Detector/BooleanCases.php'], [ + [ + 'Parameter $hasSecret in Tests\\Fixtures\\Detector\\BooleanCases::authenticateUnion might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 36, + ], + [ + 'Parameter $IsPassword in Tests\\Fixtures\\Detector\\BooleanCases::checkFlag might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 43, + ], + [ + 'Parameter $password in Tests\\Fixtures\\Detector\\BooleanCases::login might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 49, + ], + ]); +}); diff --git a/tests/Detector/CustomKeywordsTest.php b/tests/Detector/CustomKeywordsTest.php new file mode 100644 index 0000000..50bb44a --- /dev/null +++ b/tests/Detector/CustomKeywordsTest.php @@ -0,0 +1,54 @@ +rule = new SensitiveParameterDetectorRule(['banking', 'medical']); +}); + +it('detects custom keywords correctly', function () { + $this->analyse([__DIR__.'/../Fixtures/Detector/CustomKeywords.php'], [ + [ + 'Parameter $banking in Tests\\Fixtures\\Detector\\CustomKeywords::bankingMethod might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 16, + ], + [ + 'Parameter $bankingInfo in Tests\\Fixtures\\Detector\\CustomKeywords::bankingMethod might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 16, + ], + [ + 'Parameter $medical in Tests\\Fixtures\\Detector\\CustomKeywords::medicalMethod might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 21, + ], + [ + 'Parameter $medicalRecord in Tests\\Fixtures\\Detector\\CustomKeywords::medicalMethod might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 21, + ], + [ + 'Parameter $banking in Tests\\Fixtures\\Detector\\CustomKeywords::mixedKeywords might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 27, + ], + [ + 'Parameter $userBanking in Tests\\Fixtures\\Detector\\CustomKeywords::customCompounds might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 33, + ], + [ + 'Parameter $patientMedical in Tests\\Fixtures\\Detector\\CustomKeywords::customCompounds might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 33, + ], + [ + 'Parameter $Banking in Tests\\Fixtures\\Detector\\CustomKeywords::customCaseVariations might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 39, + ], + [ + 'Parameter $MEDICAL in Tests\\Fixtures\\Detector\\CustomKeywords::customCaseVariations might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 39, + ], + [ + 'Parameter $medical in Tests\\Fixtures\\Detector\\CustomKeywords::parameterProtectedCustom might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', + 58, + ], + ]); +}); diff --git a/tests/Feature/IgnoreCommentsTest.php b/tests/Detector/IgnoreCommentsTest.php similarity index 89% rename from tests/Feature/IgnoreCommentsTest.php rename to tests/Detector/IgnoreCommentsTest.php index f815190..e9137a0 100644 --- a/tests/Feature/IgnoreCommentsTest.php +++ b/tests/Detector/IgnoreCommentsTest.php @@ -3,7 +3,7 @@ declare(strict_types=1); it('detects normal parameters and respects ignore comments', function () { - $this->analyse([__DIR__.'/../Fixtures/IgnoreComments.php'], [ + $this->analyse([__DIR__.'/../Fixtures/Detector/IgnoreComments.php'], [ [ 'Parameter $password in IgnoreComments::normalFunction might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', 8, diff --git a/tests/TestCase.php b/tests/DetectorTestCase.php similarity index 75% rename from tests/TestCase.php rename to tests/DetectorTestCase.php index 5ab281c..785f783 100644 --- a/tests/TestCase.php +++ b/tests/DetectorTestCase.php @@ -4,13 +4,13 @@ namespace Tests; -use BuiltFast\Rules\SensitiveParameterDetectorRule; +use LycheeOrg\PHPStan\Rules\SensitiveParameterDetectorRule; use PHPStan\Testing\RuleTestCase; /** * @extends RuleTestCase */ -abstract class TestCase extends RuleTestCase +abstract class DetectorTestCase extends RuleTestCase { protected SensitiveParameterDetectorRule $rule; diff --git a/tests/Feature/BasicSensitiveParametersTest.php b/tests/Feature/BasicSensitiveParametersTest.php deleted file mode 100644 index 3c4f180..0000000 --- a/tests/Feature/BasicSensitiveParametersTest.php +++ /dev/null @@ -1,168 +0,0 @@ -rule = new SensitiveParameterDetectorRule(); -}); - -it('detects basic sensitive parameters', function () { - $this->analyse([__DIR__.'/../Fixtures/BasicSensitiveParameters.php'], [ - [ - 'Parameter $userSecret in Tests\\Fixtures\\BasicSensitiveParameters::regularFunction might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 13, - ], - [ - 'Parameter $credential in Tests\\Fixtures\\BasicSensitiveParameters::regularFunction might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 13, - ], - [ - 'Parameter $password in Tests\\Fixtures\\BasicSensitiveParameters::authenticate might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 19, - ], - [ - 'Parameter $apikey in Tests\\Fixtures\\BasicSensitiveParameters::setApiCredentials might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 24, - ], - [ - 'Parameter $apisecret in Tests\\Fixtures\\BasicSensitiveParameters::setApiCredentials might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 24, - ], - [ - 'Parameter $cardNumber in Tests\\Fixtures\\BasicSensitiveParameters::processPayment might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 29, - ], - [ - 'Parameter $cvv in Tests\\Fixtures\\BasicSensitiveParameters::processPayment might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 29, - ], - [ - 'Parameter $creditCardToken in Tests\\Fixtures\\BasicSensitiveParameters::processPayment might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 29, - ], - [ - 'Parameter $ssn in Tests\\Fixtures\\BasicSensitiveParameters::storeUserData might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 34, - ], - [ - 'Parameter $privateKey in Tests\\Fixtures\\BasicSensitiveParameters::storeUserData might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 34, - ], - [ - 'Parameter $authToken in Tests\\Fixtures\\BasicSensitiveParameters::handleTokens might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 39, - ], - [ - 'Parameter $refreshToken in Tests\\Fixtures\\BasicSensitiveParameters::handleTokens might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 39, - ], - [ - 'Parameter $accessToken in Tests\\Fixtures\\BasicSensitiveParameters::handleTokens might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 39, - ], - [ - 'Parameter $password in globalAuthFunction might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 46, - ], - ]); -}); - -test('does not warn for protected sensitive parameters', function () { - $this->analyse([__DIR__.'/../Fixtures/ProtectedSensitiveParameters.php'], [ - [ - 'Parameter $apikey in Tests\\Fixtures\\ProtectedSensitiveParameters::setApiCredentials might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 22, - ], - [ - 'Parameter $cvv in Tests\\Fixtures\\ProtectedSensitiveParameters::processPayment might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 28, - ], - ]); -}); - -test('detects edge cases correctly', function () { - $this->analyse([__DIR__.'/../Fixtures/EdgeCases.php'], [ - [ - 'Parameter $apikey in Tests\\Fixtures\\EdgeCases::__construct might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 13, - ], - [ - 'Parameter $credential in Tests\\Fixtures\\EdgeCases::staticMethod might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 18, - ], - [ - 'Parameter $userPassword in Tests\\Fixtures\\EdgeCases::partialMatches might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 24, - ], - [ - 'Parameter $secretKey in Tests\\Fixtures\\EdgeCases::partialMatches might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 24, - ], - [ - 'Parameter $apiToken in Tests\\Fixtures\\EdgeCases::partialMatches might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 24, - ], - [ - 'Parameter $Password in Tests\\Fixtures\\EdgeCases::caseVariations might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 30, - ], - [ - 'Parameter $SECRET in Tests\\Fixtures\\EdgeCases::caseVariations might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 30, - ], - [ - 'Parameter $Token in Tests\\Fixtures\\EdgeCases::caseVariations might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 30, - ], - [ - 'Parameter $myPassword in Tests\\Fixtures\\EdgeCases::mixedCaseCompounds might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 36, - ], - [ - 'Parameter $userSecret in Tests\\Fixtures\\EdgeCases::mixedCaseCompounds might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 36, - ], - [ - 'Parameter $appToken in Tests\\Fixtures\\EdgeCases::mixedCaseCompounds might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 36, - ], - [ - 'Parameter $passwordless in Tests\\Fixtures\\EdgeCases::falsePositives might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 42, - ], - [ - 'Parameter $secretion in Tests\\Fixtures\\EdgeCases::falsePositives might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 42, - ], - [ - 'Parameter $password in Tests\\Fixtures\\EdgeCases::multiplePasswords might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 49, - ], - [ - 'Parameter $confirmPassword in Tests\\Fixtures\\EdgeCases::multiplePasswords might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 49, - ], - [ - 'Parameter $oldPassword in Tests\\Fixtures\\EdgeCases::multiplePasswords might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 49, - ], - [ - 'Parameter $password123 in Tests\\Fixtures\\EdgeCases::unusualCases might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 55, - ], - [ - 'Parameter $password_hash in Tests\\Fixtures\\EdgeCases::unusualCases might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 55, - ], - [ - 'Parameter $secret in Tests\\Fixtures\\EdgeCases::privateMethod might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 61, - ], - [ - 'Parameter $token in Tests\\Fixtures\\EdgeCases::protectedMethod might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 66, - ], - ]); -}); diff --git a/tests/Feature/CustomKeywordsTest.php b/tests/Feature/CustomKeywordsTest.php deleted file mode 100644 index 8f18bc1..0000000 --- a/tests/Feature/CustomKeywordsTest.php +++ /dev/null @@ -1,54 +0,0 @@ -rule = new SensitiveParameterDetectorRule(['banking', 'medical']); -}); - -it('detects custom keywords correctly', function () { - $this->analyse([__DIR__.'/../Fixtures/CustomKeywords.php'], [ - [ - 'Parameter $banking in Tests\\Fixtures\\CustomKeywords::bankingMethod might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 16, - ], - [ - 'Parameter $bankingInfo in Tests\\Fixtures\\CustomKeywords::bankingMethod might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 16, - ], - [ - 'Parameter $medical in Tests\\Fixtures\\CustomKeywords::medicalMethod might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 21, - ], - [ - 'Parameter $medicalRecord in Tests\\Fixtures\\CustomKeywords::medicalMethod might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 21, - ], - [ - 'Parameter $banking in Tests\\Fixtures\\CustomKeywords::mixedKeywords might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 27, - ], - [ - 'Parameter $userBanking in Tests\\Fixtures\\CustomKeywords::customCompounds might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 33, - ], - [ - 'Parameter $patientMedical in Tests\\Fixtures\\CustomKeywords::customCompounds might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 33, - ], - [ - 'Parameter $Banking in Tests\\Fixtures\\CustomKeywords::customCaseVariations might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 39, - ], - [ - 'Parameter $MEDICAL in Tests\\Fixtures\\CustomKeywords::customCaseVariations might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 39, - ], - [ - 'Parameter $medical in Tests\\Fixtures\\CustomKeywords::parameterProtectedCustom might contain sensitive information. Add the #[\\SensitiveParameter] attribute or ignore with `@phpstan-ignore sensitiveParameter.missing`.', - 58, - ], - ]); -}); diff --git a/tests/Fixtures/BasicSensitiveParameters.php b/tests/Fixtures/Detector/BasicSensitiveParameters.php similarity index 97% rename from tests/Fixtures/BasicSensitiveParameters.php rename to tests/Fixtures/Detector/BasicSensitiveParameters.php index 4882f40..c2d63b8 100644 --- a/tests/Fixtures/BasicSensitiveParameters.php +++ b/tests/Fixtures/Detector/BasicSensitiveParameters.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Tests\Fixtures; +namespace Tests\Fixtures\Detector; /** * Test fixture with basic sensitive parameters that should trigger warnings diff --git a/tests/Fixtures/Detector/BooleanCases.php b/tests/Fixtures/Detector/BooleanCases.php new file mode 100644 index 0000000..d969b0c --- /dev/null +++ b/tests/Fixtures/Detector/BooleanCases.php @@ -0,0 +1,53 @@ +nonSensitiveMethod($password); + } + + // $password is marked sensitive here and forwarded into a callee that is also marked sensitive - should NOT trigger warning + public function protectedByCalleeParameterAttribute(#[SensitiveParameter] string $password): void + { + $this->sensitiveMethod($password); + } + + // $password is marked sensitive here and forwarded into a callee protected via a function-level attribute - should NOT trigger warning + public function protectedByCalleeFunctionLevelAttribute(#[SensitiveParameter] string $password): void + { + $this->functionLevelProtectedMethod($password); + } + + // $password isn't marked sensitive here, so there's nothing to propagate even though the callee's parameter is sensitive - should NOT trigger warning + public function nonSensitiveParameterIsNotFlagged(string $password): void + { + $this->sensitiveMethod($password); + } + + // $password is concatenated before being passed on, so it's no longer a bare variable pass-through - should NOT trigger warning + public function transformedValueIsNotFlagged(#[SensitiveParameter] string $password): void + { + $this->nonSensitiveMethod($password.'-suffix'); + } + + // $other isn't a parameter of this function at all, so there's nothing to propagate - should NOT trigger warning + public function unrelatedLocalVariableIsNotFlagged(): void + { + $other = 'value'; + $this->nonSensitiveMethod($other); + } + + // $token is marked sensitive here but forwarded into a constructor whose parameter isn't - should trigger warning + public function missingPropagationOnConstructorCall(#[SensitiveParameter] string $token): void + { + new NonSensitiveConsumer($token); + } + + // $secret is marked sensitive here but forwarded into a static call whose parameter isn't - should trigger warning + public function missingPropagationOnStaticCall(#[SensitiveParameter] string $secret): void + { + self::nonSensitiveStaticMethod($secret); + } + + // $apiKey is marked sensitive here but forwarded into a plain function whose parameter isn't - should trigger warning + public function missingPropagationOnFunctionCall(#[SensitiveParameter] string $apiKey): void + { + propagationGlobalNonSensitiveFunction($apiKey); + } + + // $secret is marked sensitive here and passed as an unknown named argument, which is + // collected into the callee's unprotected variadic parameter - should trigger warning + public function missingPropagationOnNamedVariadicCall(#[SensitiveParameter] string $secret): void + { + $this->nonSensitiveVariadicMethod(secret: $secret); + } + + // $password is marked sensitive here but reassigned to a literal before being forwarded, so + // it no longer provably holds the original sensitive value - should NOT trigger warning + public function reassignedParameterIsNotFlagged(#[SensitiveParameter] string $password): void + { + $password = 'redacted'; + $this->nonSensitiveMethod($password); + } + + // Helper callee whose parameter is NOT sensitive + public function nonSensitiveMethod(string $password): void + { + } + + // Helper callee whose parameter IS sensitive + public function sensitiveMethod(#[SensitiveParameter] string $password): void + { + } + + // Helper callee protected via a function-level attribute + #[SensitiveParameter] + public function functionLevelProtectedMethod(string $password): void + { + } + + // Helper callee whose variadic catch-all parameter is NOT sensitive + public function nonSensitiveVariadicMethod(mixed ...$values): void + { + } +} + +// Helper class whose constructor parameter is NOT sensitive, used by missingPropagationOnConstructorCall() +final class NonSensitiveConsumer +{ + public function __construct(string $token) + { + } +} + +// Helper global function whose parameter is NOT sensitive, used by missingPropagationOnFunctionCall() +function propagationGlobalNonSensitiveFunction(string $apiKey): void +{ +} diff --git a/tests/Fixtures/Storage/StorageCases.php b/tests/Fixtures/Storage/StorageCases.php new file mode 100644 index 0000000..d221bfd --- /dev/null +++ b/tests/Fixtures/Storage/StorageCases.php @@ -0,0 +1,91 @@ +password = $password; + } + + // Sensitive parameter wrapped in \SensitiveParameterValue before storage - should NOT trigger warning + public function wrappedAssignmentIsNotFlagged(#[SensitiveParameter] string $password): void + { + $this->wrappedPassword = new SensitiveParameterValue($password); + } + + // Sensitive parameter wrapped in \SensitiveParameterValue but unwrapped via getValue() before storage - should trigger warning + public function wrappedAssignmentIsFlagged(SensitiveParameterValue $password): void + { + $this->password = $password->getValue(); + } + + // Function-level SensitiveParameter also makes $password sensitive, so unwrapped storage should still trigger warning + #[SensitiveParameter] + public function functionLevelAttributeIsAlsoDetected(string $password): void + { + $this->password = $password; + } + + // Sensitive parameter assigned directly to a static property, unwrapped - should trigger warning + public function unwrappedStaticAssignmentIsFlagged(#[SensitiveParameter] string $secret): void + { + self::$staticSecret = $secret; + } + + // Non-sensitive parameter stored directly - should NOT trigger warning + public function nonSensitiveAssignmentIsNotFlagged(string $username): void + { + $this->password = $username; + } + + // Sensitive parameter is transformed before storage, so it's no longer a bare variable pass-through - should NOT trigger warning + public function transformedValueIsNotFlagged(#[SensitiveParameter] string $password): void + { + $this->password = mb_trim($password); + } + + // Sensitive parameter is unwrapped into a temporary variable before storage; the rule only + // matches a direct `$var->getValue()` call, so routing through an intermediate variable + // defeats detection - should NOT trigger warning (known limitation, not an endorsed pattern) + public function temporaryVariableIndirectionIsNotFlagged(SensitiveParameterValue $password): void + { + $passwordValue = $password->getValue(); + $this->password = $passwordValue; + } + + // Sensitive parameter is reassigned before storage, so it no longer provably holds the + // original sensitive value - should NOT trigger warning + public function reassignedParameterIsNotFlagged(#[SensitiveParameter] string $password): void + { + $password = 'redacted'; + $this->password = $password; + } +} diff --git a/tests/Fixtures/Types/SensitiveParameterValueTypes.php b/tests/Fixtures/Types/SensitiveParameterValueTypes.php new file mode 100644 index 0000000..c32fc7c --- /dev/null +++ b/tests/Fixtures/Types/SensitiveParameterValueTypes.php @@ -0,0 +1,56 @@ +', $value); + assertType('string', $value->getValue()); + } + + public function intValue(int $pin): void + { + $value = new SensitiveParameterValue($pin); + + assertType('SensitiveParameterValue', $value); + assertType('int', $value->getValue()); + } + + public function nullableValue(?string $token): void + { + $value = new SensitiveParameterValue($token); + + assertType('SensitiveParameterValue', $value); + assertType('string|null', $value->getValue()); + } + + /** + * @param array $credentials + */ + public function arrayValue(array $credentials): void + { + $value = new SensitiveParameterValue($credentials); + + assertType('SensitiveParameterValue>', $value); + assertType('array', $value->getValue()); + } + + public function objectValue(stdClass $secret): void + { + $value = new SensitiveParameterValue($secret); + + assertType('SensitiveParameterValue', $value); + assertType('stdClass', $value->getValue()); + } +} diff --git a/tests/Pest.php b/tests/Pest.php index 1359f4b..dbdf63c 100644 --- a/tests/Pest.php +++ b/tests/Pest.php @@ -1,7 +1,9 @@ in('Feature'); +uses(DetectorTestCase::class)->in('Detector'); +uses(PropagationTestCase::class)->in('Propagation'); +uses(StorageTestCase::class)->in('Storage'); /* |-------------------------------------------------------------------------- diff --git a/tests/Propagation/SensitiveParameterPropagationTest.php b/tests/Propagation/SensitiveParameterPropagationTest.php new file mode 100644 index 0000000..258e58c --- /dev/null +++ b/tests/Propagation/SensitiveParameterPropagationTest.php @@ -0,0 +1,28 @@ +analyse([__DIR__.'/../Fixtures/Propagation/PropagationCases.php'], [ + [ + 'Parameter $password is marked #[\\SensitiveParameter] but is passed to a parameter ($password) that is not itself marked with #[\\SensitiveParameter]. Add the attribute there too or ignore with `@phpstan-ignore sensitiveParameter.propagation`.', + 24, + ], + [ + 'Parameter $token is marked #[\\SensitiveParameter] but is passed to a parameter ($token) that is not itself marked with #[\\SensitiveParameter]. Add the attribute there too or ignore with `@phpstan-ignore sensitiveParameter.propagation`.', + 61, + ], + [ + 'Parameter $secret is marked #[\\SensitiveParameter] but is passed to a parameter ($secret) that is not itself marked with #[\\SensitiveParameter]. Add the attribute there too or ignore with `@phpstan-ignore sensitiveParameter.propagation`.', + 67, + ], + [ + 'Parameter $apiKey is marked #[\\SensitiveParameter] but is passed to a parameter ($apiKey) that is not itself marked with #[\\SensitiveParameter]. Add the attribute there too or ignore with `@phpstan-ignore sensitiveParameter.propagation`.', + 73, + ], + [ + 'Parameter $secret is marked #[\\SensitiveParameter] but is passed to a parameter ($values) that is not itself marked with #[\\SensitiveParameter]. Add the attribute there too or ignore with `@phpstan-ignore sensitiveParameter.propagation`.', + 80, + ], + ]); +}); diff --git a/tests/PropagationTestCase.php b/tests/PropagationTestCase.php new file mode 100644 index 0000000..cc7b633 --- /dev/null +++ b/tests/PropagationTestCase.php @@ -0,0 +1,21 @@ + + */ +abstract class PropagationTestCase extends RuleTestCase +{ + protected SensitiveParameterPropagationRule $rule; + + protected function getRule(): SensitiveParameterPropagationRule + { + return $this->rule ?? new SensitiveParameterPropagationRule($this->createReflectionProvider()); + } +} diff --git a/tests/Storage/SensitiveParameterStorageTest.php b/tests/Storage/SensitiveParameterStorageTest.php new file mode 100644 index 0000000..a92ffc4 --- /dev/null +++ b/tests/Storage/SensitiveParameterStorageTest.php @@ -0,0 +1,28 @@ +analyse([__DIR__.'/../Fixtures/Storage/StorageCases.php'], [ + [ + 'Promoted property $promotedPassword receives a sensitive value but assigns it directly, bypassing \\SensitiveParameterValue. Declare the property without promotion and assign `new \\SensitiveParameterValue($promotedPassword)` in the constructor body, or ignore with `@phpstan-ignore sensitiveParameter.unwrappedPromotion`.', + 24, + ], + [ + '$password is a sensitive value and must be wrapped in \\SensitiveParameterValue before being stored, e.g. `... = new \\SensitiveParameterValue($password);`. Ignore with `@phpstan-ignore sensitiveParameter.unwrappedStorage`.', + 35, + ], + [ + '$password->getValue() unwraps a \\SensitiveParameterValue and must not be stored raw. Store $password itself instead of calling getValue() on it. Ignore with `@phpstan-ignore sensitiveParameter.unwrappedGetValue`.', + 47, + ], + [ + '$password is a sensitive value and must be wrapped in \\SensitiveParameterValue before being stored, e.g. `... = new \\SensitiveParameterValue($password);`. Ignore with `@phpstan-ignore sensitiveParameter.unwrappedStorage`.', + 54, + ], + [ + '$secret is a sensitive value and must be wrapped in \\SensitiveParameterValue before being stored, e.g. `... = new \\SensitiveParameterValue($secret);`. Ignore with `@phpstan-ignore sensitiveParameter.unwrappedStorage`.', + 60, + ], + ]); +}); diff --git a/tests/StorageTestCase.php b/tests/StorageTestCase.php new file mode 100644 index 0000000..c7e88aa --- /dev/null +++ b/tests/StorageTestCase.php @@ -0,0 +1,21 @@ + + */ +abstract class StorageTestCase extends RuleTestCase +{ + protected SensitiveParameterStorageRule $rule; + + protected function getRule(): SensitiveParameterStorageRule + { + return $this->rule ?? new SensitiveParameterStorageRule(); + } +} diff --git a/tests/Types/SensitiveParameterValueTypesTest.php b/tests/Types/SensitiveParameterValueTypesTest.php new file mode 100644 index 0000000..b147750 --- /dev/null +++ b/tests/Types/SensitiveParameterValueTypesTest.php @@ -0,0 +1,36 @@ + + */ + public static function dataFileAsserts(): iterable + { + yield from self::gatherAssertTypes(__DIR__.'/../Fixtures/Types/SensitiveParameterValueTypes.php'); + } + + /** + * @return string[] + */ + public static function getAdditionalConfigFiles(): array + { + return [__DIR__.'/../../extension.neon']; + } + + /** + * @param mixed ...$args + */ + #[DataProvider('dataFileAsserts')] + public function testFileAsserts(string $assertType, string $file, ...$args): void + { + $this->assertFileAsserts($assertType, $file, ...$args); + } +} diff --git a/tests/Unit/Rules/SensitiveParameterDetectorRuleTest.php b/tests/Unit/Rules/SensitiveParameterDetectorRuleTest.php index 534b723..a07260c 100644 --- a/tests/Unit/Rules/SensitiveParameterDetectorRuleTest.php +++ b/tests/Unit/Rules/SensitiveParameterDetectorRuleTest.php @@ -2,7 +2,7 @@ declare(strict_types=1); -use BuiltFast\Rules\SensitiveParameterDetectorRule; +use LycheeOrg\PHPStan\Rules\SensitiveParameterDetectorRule; use PhpParser\Node\FunctionLike; use PHPStan\Rules\Rule;