chore(deps): bump pinned dependencies to clear security advisories - #153
Open
smarcet wants to merge 1 commit into
Open
chore(deps): bump pinned dependencies to clear security advisories#153smarcet wants to merge 1 commit into
smarcet wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThe pull request updates three pinned Composer dependencies, adjusts the custom boolean validator signature, and adds a completed plan with upgrade verification and advisory audit results. ChangesDependency upgrade
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-153/ This page is automatically updated on each push to this PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Validators/CustomValidator.php (1)
46-56: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle
boolean:strictin the active validator.
CustomValidator::validateBoolean()ignores$parameters, soboolean:strictwould accept strings and integers. No current rule usesboolean:strict, but the custom validator is active.Remove the default from
$parameters. Delegate thestrictbranch toparent::validateBoolean(...). Preserve existing conversions for unqualifiedbooleanrules.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Validators/CustomValidator.php` around lines 46 - 56, Update CustomValidator::validateBoolean() to require the $parameters argument and delegate validation to parent::validateBoolean(...) when the strict parameter is present. Preserve the current string, integer, and boolean acceptance behavior for unqualified boolean rules.Sources: MCP tools, Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/plans/2026-08-11-bump-pinned-deps-security-advisories.md`:
- Around line 85-90: Update the verification checklist items in the Task 1 and
Task 2 sections to reflect the reported successful results, marking each
completed composer constraint, lockfile, package-preservation, and validation
check as [x]. If any check lacks evidence, leave it unchecked and explicitly
identify it as incomplete rather than retaining an inconsistent COMPLETE status.
- Line 28: Reconcile the security-advisory summary with the authoritative
composer audit --locked result: update the line-28 list, the remaining
symfony/cache entry around the documented audit details, and all affected
advisory totals so symfony/cache is consistently classified as cleared or
remaining.
- Line 179: The recorded status summary incorrectly treats the plan file as
unrelated. Update the status-check description in the plan to include
docs/plans/2026-08-11-bump-pinned-deps-security-advisories.md as a changed file
while distinguishing the unrelated pre-existing untracked directories, and
ensure the listed files match the actual git status.
- Line 141: Add a regression check covering OTEL-enabled tests when the
configured OTLP collector is unreachable, using the documented
otel-collector:4318 configuration and an unavailable endpoint scenario. Verify
the dependency bump does not produce Doctrine cleanup or foreign-key errors, or
isolate exporter retry behavior from Doctrine cleanup so the test remains
stable. Update the documented finding only after this behavior is validated.
---
Outside diff comments:
In `@app/Validators/CustomValidator.php`:
- Around line 46-56: Update CustomValidator::validateBoolean() to require the
$parameters argument and delegate validation to parent::validateBoolean(...)
when the strict parameter is present. Preserve the current string, integer, and
boolean acceptance behavior for unqualified boolean rules.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7fc113da-759a-41a7-bf75-8e2a8fe8a94b
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
app/Validators/CustomValidator.phpcomposer.jsondocs/plans/2026-08-11-bump-pinned-deps-security-advisories.md
|
|
||
| ## Context | ||
|
|
||
| Dependabot has open advisories against openstackid's PHP dependencies, including 8 high-severity ones covered by this bump: `laravel/framework < 12.60.0` (high) and `guzzlehttp/guzzle < 7.15.2` (high), plus medium/low items cleared as transitive side effects (`guzzlehttp/psr7 < 2.12.3`, `symfony/mime`, `symfony/http-foundation`, `symfony/routing`, `symfony/mailer`, `symfony/cache`, `symfony/polyfill-intl-idn`, `league/commonmark < 2.9.0`, `symfony/process`). |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Correct the contradictory symfony/cache audit result.
Line 28 lists symfony/cache among advisories cleared by the bump. Lines 173-177 list one symfony/cache advisory as remaining. Reconcile the list and totals with one composer audit --locked result.
Also applies to: 173-177
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/plans/2026-08-11-bump-pinned-deps-security-advisories.md` at line 28,
Reconcile the security-advisory summary with the authoritative composer audit
--locked result: update the line-28 list, the remaining symfony/cache entry
around the documented audit details, and all affected advisory totals so
symfony/cache is consistently classified as cleared or remaining.
Comment on lines
+85
to
+90
| - [ ] The three constraints in `composer.json` read `12.62.0`, `7.15.2`, `v14.5.3`; no other `require`/`require-dev` line changed. | ||
| - [ ] `composer.lock` regenerated with `laravel/framework v12.62.0`, `guzzlehttp/guzzle 7.15.2`, `vladimir-yuldashev/laravel-queue-rabbitmq v14.5.3`. | ||
| - [ ] `phpseclib/phpseclib` still at `3.0.43` and `smarcet/jose4php` still resolvable (the hard pin was not broken). | ||
| - [x] Verify: `composer validate && composer install --dry-run` exits 0. | ||
|
|
||
| **Task 1 result (completed):** constraints at `12.62.0` / `7.15.2` / `v14.5.3`; lock regenerated (861 insertions, 682 deletions); `webmozart/assert` removed, `symfony/polyfill-php85` added; `phpseclib 3.0.43` and `smarcet/jose4php 2.0.0` intact. Installed versions confirmed in `vendor/composer/installed.php`. `composer install --dry-run` → "Nothing to install, update or remove"; `composer validate` → exit 0. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Mark completed verification items as complete.
The plan says Status: COMPLETE, but the Task 1 checks at Lines 85-87 and the Task 2 checks at Lines 126-128 remain unchecked. Later result sections report that these checks passed. Mark them [x], or state which checks are still incomplete.
Also applies to: 124-130
🧰 Tools
🪛 LanguageTool
[grammar] ~90-~90: Ensure spelling is correct
Context: ...ll-php85added;phpseclib 3.0.43andsmarcet/jose4php 2.0.0intact. Installed versions confirmed invendor...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/plans/2026-08-11-bump-pinned-deps-security-advisories.md` around lines
85 - 90, Update the verification checklist items in the Task 1 and Task 2
sections to reflect the reported successful results, marking each completed
composer constraint, lockfile, package-preservation, and validation check as
[x]. If any check lacks evidence, leave it unchecked and explicitly identify it
as incomplete rather than retaining an inconsistent COMPLETE status.
| - OTEL Functionality Test Suite: `OK (23 tests, 52 assertions)`. | ||
| - OTEL Custom Formatters Test Suite: `OK (12 tests, 49 assertions)`. | ||
| - **Boot smoke:** `php artisan about` (Laravel Version 12.62.0, exit 0), `php artisan route:list` (178 routes, exit 0), `php artisan config:clear` (exit 0). | ||
| - **Regression-attribution finding (investigated, not a blocker):** with OTEL left enabled and the local `otel-collector` unreachable at the configured endpoint (`localhost:4318` from inside the app container — a pre-existing local dev misconfiguration, collector is actually reachable at `otel-collector:4318`), the bumped Guzzle/promises' export-retry timing triggered 18 spurious test errors (FK violations from Doctrine `UnitOfWork` flushes leaking across `TestSeeder` cleanup ordering). Isolated via an old-deps baseline run in the identical container/DB (same fresh-schema procedure): 0 errors with OTEL enabled on old deps, 18 errors with OTEL enabled on bumped deps, 0 errors on bumped deps once OTEL is disabled. Since CI (`pull_request_unit_tests.yml`, `nightly_unit_tests.yml`) always sets `OTEL_SDK_DISABLED=true`/`OTEL_SERVICE_ENABLED=false`, this local-only condition never reaches CI and is not part of this plan's scope — noted here for visibility, not filed as a task. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 3 \
'OTEL_SDK_DISABLED|OTEL_SERVICE_ENABLED|OTEL_EXPORTER_OTLP|localhost:4318|otel-collector:4318' \
. --glob '!vendor/**' || trueRepository: OpenStackweb/openstackid
Length of output: 8015
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- workflow OTEL settings ---'
rg -n -C 5 \
'pull_request_unit_tests|nightly_unit_tests|OTEL_SDK_DISABLED|OTEL_SERVICE_ENABLED|OTEL_EXPORTER_OTLP_ENDPOINT' \
.github . --glob '*.yml' --glob '*.yaml' --glob '!vendor/**' || true
printf '%s\n' '--- OTEL configuration and package wiring ---'
sed -n '1,190p' config/opentelemetry.php
sed -n '145,190p' config/app.php
sed -n '1,100p' app/Http/Middleware/TrackRequestMiddleware.php
rg -n -C 4 \
'OpenTelemetry|OTLP|otlp|otel-collector|4318|OTEL_' \
composer.json docker-compose.yml docker-compose.*.yml .env* docs app config tests .github \
--glob '!vendor/**' 2>/dev/null || true
printf '%s\n' '--- relevant plan context ---'
sed -n '120,185p' docs/plans/2026-08-11-bump-pinned-deps-security-advisories.mdRepository: OpenStackweb/openstackid
Length of output: 50380
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- application and collector services ---'
sed -n '1,115p' docker-compose.yml
sed -n '115,155p' .env.example
sed -n '1,75p' .github/workflows/pull_request_unit_tests.yml
sed -n '1,75p' .github/workflows/push.yml
printf '%s\n' '--- test bootstrap and cleanup paths ---'
rg -n -C 8 \
'TestSeeder|tearDown|UnitOfWork|foreign key|Schema::|migrate|fresh|OTEL_SDK_DISABLED|opentelemetry.enabled' \
tests phpunit.xml* database app \
--glob '!vendor/**' \
--glob '*.php' \
--glob 'phpunit.xml*' | head -n 500
printf '%s\n' '--- dependency versions and OTEL package references ---'
rg -n -C 2 \
'guzzlehttp/guzzle|guzzlehttp/promises|open-telemetry|keepsuit/laravel-open-telemetry' \
composer.json composer.lockRepository: OpenStackweb/openstackid
Length of output: 50380
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
compose = Path("docker-compose.yml").read_text()
env_example = Path(".env.example").read_text()
otel_config = Path("config/opentelemetry.php").read_text()
workflow_files = [
Path(".github/workflows/pull_request_unit_tests.yml"),
Path(".github/workflows/push.yml"),
]
def service_block(name):
match = re.search(
rf"(?ms)^ {re.escape(name)}:\n(.*?)(?=^ [A-Za-z0-9_-]+:\n|\Z)",
compose,
)
assert match, f"missing service: {name}"
return match.group(1)
app = service_block("app")
collector = service_block("otel-collector")
assert "idp-local-net" in app
assert "idp-local-net" in collector
assert "OTEL_EXPORTER_OTLP_ENDPOINT=http://otel-collector:4318" in env_example
assert "'endpoint' => env(Variables::OTEL_EXPORTER_OTLP_ENDPOINT, 'http://localhost:4318')" in otel_config
for workflow in workflow_files:
text = workflow.read_text()
assert re.search(r"(?m)^\s+OTEL_SDK_DISABLED:\s+true\s*$", text)
assert re.search(r"(?m)^\s+OTEL_SERVICE_ENABLED:\s+false\s*$", text)
print("compose: app and otel-collector share idp-local-net")
print("documented endpoint: http://otel-collector:4318")
print("application fallback endpoint: http://localhost:4318")
print("CI workflows: OTEL_SDK_DISABLED=true and OTEL_SERVICE_ENABLED=false")
print("plan-reported outage result: 18 errors after the bump, 0 with OTEL disabled")
PYRepository: OpenStackweb/openstackid
Length of output: 459
Add a collector-failure regression check.
The documented Docker configuration uses http://otel-collector:4318, so localhost:4318 is a local configuration error. The dependency bump still causes 18 test errors when an OTLP collector is unreachable, and CI does not cover this OTEL-enabled path. Add a regression check or isolate exporter retries from Doctrine cleanup before treating this behavior as out of scope.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/plans/2026-08-11-bump-pinned-deps-security-advisories.md` at line 141,
Add a regression check covering OTEL-enabled tests when the configured OTLP
collector is unreachable, using the documented otel-collector:4318 configuration
and an unavailable endpoint scenario. Verify the dependency bump does not
produce Doctrine cleanup or foreign-key errors, or isolate exporter retry
behavior from Doctrine cleanup so the test remains stable. Update the documented
finding only after this behavior is validated.
|
|
||
| Remaining advisories (unchanged before/after, out of scope per the plan's Context section): `aws/aws-sdk-php` (2), `firebase/php-jwt` (1, needs 6.x→7.x major), `google/protobuf` (1), `phpseclib/phpseclib` (4, blocked by `smarcet/jose4php` 2.0.0's exact pin), `phpunit/phpunit` (1, dev-only), `psy/psysh` (1, dev-only), `symfony/cache` (1), `symfony/dom-crawler` (1, dev-only), `symfony/yaml` (3). | ||
|
|
||
| `git status --short` diff: `app/Validators/CustomValidator.php`, `composer.json`, `composer.lock` — exactly the plan's `Files:` plus the Laravel-compat fix documented in Task 2. Pre-existing untracked dirs (`.codegraph/`, `.codex/`, `docs/`, `tests/e2e/`) unrelated to this change. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Reconcile the recorded changed-file list.
The reviewed PR includes docs/plans/2026-08-11-bump-pinned-deps-security-advisories.md, but Line 179 describes docs/ as pre-existing and unrelated and lists only the application and Composer files. Re-run the status check and distinguish this plan from unrelated untracked directories.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/plans/2026-08-11-bump-pinned-deps-security-advisories.md` at line 179,
The recorded status summary incorrectly treats the plan file as unrelated.
Update the status-check description in the plan to include
docs/plans/2026-08-11-bump-pinned-deps-security-advisories.md as a changed file
while distinguishing the unrelated pre-existing untracked directories, and
ensure the listed files match the actual git status.
Port the dependency bump from summit-api PR #570 to openstackid: - laravel/framework 12.0 -> 12.62.0 - guzzlehttp/guzzle 7.9.3 -> 7.15.2 (7.12.1 in the source PR; bumped further here to also clear a newer high-severity guzzle advisory) - vladimir-yuldashev/laravel-queue-rabbitmq v14.2.0 -> v14.5.3 (Consumer::stop() must match the Illuminate\Queue\Worker::stop() signature shipped in Laravel 12.62) Also fixes a Laravel 12.62 compat break: CustomValidator::validateBoolean() override widened to accept $parameters, matching the new base Illuminate\Validation\Validator::validateBoolean() signature (was a fatal declaration-incompatible error otherwise). Clears 34 open Dependabot advisories across laravel/framework, guzzlehttp/guzzle, guzzlehttp/psr7, league/commonmark, and several symfony packages refreshed as transitives. Remaining advisories (phpseclib, firebase/php-jwt, aws-sdk-php, google/protobuf, phpunit, psy/psysh, symfony/cache, symfony/dom-crawler, symfony/yaml) are out of scope - see the plan for why each is blocked or deferred. Full regression verified locally: default suite (177 tests) + OTEL Functionality suite (23 tests) + OTEL Custom Formatters suite (12 tests), all green. Queue worker boot-tested against local rabbitmq.
smarcet
force-pushed
the
chore/bump-pinned-deps-security-advisories
branch
from
August 12, 2026 02:14
c115599 to
a5a5caf
Compare
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-153/ This page is automatically updated on each push to this PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ref: OpenStackweb/summit-api#570 (same shape, ported to openstackid)
Resolve open Dependabot alerts by updating:
Consumer::stop()override is incompatible with theIlluminate\Queue\Worker::stop()signature shipped in Laravel 12.62)Also fixes a Laravel 12.62 compat break surfaced by the bump:
CustomValidator::validateBoolean()widened to accept$parameters, matching the new baseIlluminate\Validation\Validator::validateBoolean()signature (was a fatal declaration-incompatible error otherwise). Method body unchanged.Transitive packages refreshed within existing constraints via
-W(guzzlehttp/psr7, guzzlehttp/promises, league/commonmark, several symfony packages, etc.), clearing 34 open advisories in total.Remaining advisories (deliberately out of scope, documented in the plan):
phpseclib/phpseclib— blocked bysmarcet/jose4php2.0.0's exact pin onphpseclib 3.0.43; needs a jose4php release first.firebase/php-jwt(low) — needs the 6.x -> 7.x major in the token-validation path.aws/aws-sdk-php,google/protobuf,phpunit,psy/psysh,symfony/cache,symfony/dom-crawler,symfony/yaml— outside this bump's three-package shape; a separate sweep.Verification (local, matching CI's
OTEL_SDK_DISABLED=true OTEL_SERVICE_ENABLED=false):php artisan about(Laravel 12.62.0),route:list(178 routes),config:clear— all exit 0.php artisan queue:work rabbitmq --max-jobs=1 --stop-when-emptyagainst local rabbitmq — exit 0, noConsumer::stop()signature fatal.composer auditbefore/after: laravel/framework (4->0), guzzlehttp/guzzle (9->0), guzzlehttp/psr7 (4->0), league/commonmark (8->0), symfony http-foundation/mailer/mime/polyfill-intl-idn/process/routing (9->0 combined).Full plan with investigation notes and per-task evidence:
docs/plans/2026-08-11-bump-pinned-deps-security-advisories.md.Summary by CodeRabbit
Bug Fixes
Security
Documentation