Modernize: PHP 8.2+, Flysystem 3, GitHub Actions, and security/correctness fixes - #142
Open
tarikmanoar wants to merge 6 commits into
Open
Modernize: PHP 8.2+, Flysystem 3, GitHub Actions, and security/correctness fixes#142tarikmanoar wants to merge 6 commits into
tarikmanoar wants to merge 6 commits into
Conversation
…+ Pint - Remove dead CI configs (.travis.yml, .scrutinizer.yml, .styleci.yml) - Add GitHub Actions: test matrix (PHP 8.2/8.3/8.4 x lowest/stable), PHPStan static analysis, and Pint code-style checks - Add Dependabot for composer + github-actions updates - Add .editorconfig, phpstan.neon (level 5), pint.json (laravel preset) - Refresh .gitignore for modern tool caches Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Bump floor to PHP ^8.2; drop EOL Guzzle 6 and psr7 v1 - Require league/flysystem ^3, google/apiclient ^2.15, guzzle ^7.5, psr7 ^2.4 - Move dev tooling to PHPUnit ^10.5|^11, add phpstan + pint - Add ext-fileinfo (used by FinfoMimeTypeDetector) - Add composer scripts (test, analyse, format) and PSR-4 test autoloading - Migrate phpunit.xml.dist to the PHPUnit 10/11 schema - Restructure tests into Unit and Integration suites; namespace the live integration test under Masbug\Flysystem\Tests\Integration Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…folding
Bug fixes (surfaced by an adversarial multi-dimension audit):
- Shortcut metadata crash: add shortcutDetails to FETCHFIELDS_GET and guard
the shortcut branch in normaliseObject() so broken/unfetched shortcuts raise
UnableToReadFile instead of a fatal "property on null" error
- copy(): fetch the source object explicitly before reading its visibility
(it could be resolved purely from the path cache and be absent from the
object cache, causing getRawVisibility() on null)
- delete(): only treat a missing path as a successful no-op; genuine API
failures now surface as UnableToDeleteFile instead of being swallowed
- move(): guard the post-update fetch and throw UnableToMoveFile when it does
not yield a DriveFile (previously reported false success)
- writeData(): forward the underlying throwable as the exception cause
- hasDir(): handle getMetadata() returning false
- cachePaths(): fix broken for-loop condition (compared int to array)
- Escape file names interpolated into Google Drive query literals (prevents
query-language injection via crafted names)
Security:
- readStream(): only send the OAuth bearer token and cookies to trusted Google
hosts, enforce HTTPS + verified TLS, and guard the socket connection so the
access token can never leak to a third-party redirect target
Hardening & modernization:
- Remove the global define('DEBUG_ME') and all leftover echo debug blocks
- Add native param/return types and union types across the public API
- Reuse a shared FinfoMimeTypeDetector instead of instantiating per upload
- StreamableUpload: strict_types, typed properties, strict comparisons,
int chunk size, nullable size sentinel, drop the unused boundary property
- Fix typos and misleading comments
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds a Unit test suite (runnable in CI without Google credentials) covering: - escapeQueryValue() incl. query-injection break-out attempts - normalizeDirname(), dirname(), extension-based guessMimeType() - trusted-host checks (security regression tests for the readStream token-leak fix: suffix/prefix spoofing and lookalike domains must be rejected) - splitPath(), splitFileExtension(), sanitizeFilename(), indexString() - cleanOptParameters() global/per-operation merging Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- README: replace dead Travis/StyleCI badges with GitHub Actions + PHP-version badges; add a version/requirements compatibility matrix and updated install instructions for the 3.x (PHP 8.2+, Flysystem v3) line - Add CHANGELOG.md (3.0.0), CONTRIBUTING.md and SECURITY.md - .gitattributes: drop references to removed files and export-ignore the new dev tooling; normalize line endings - .gitignore: ignore .env and modern tool caches Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Modernizes the Flysystem Google Drive adapter to current PHP/Flysystem tooling while addressing multiple audit-driven security and correctness issues (notably around download redirects/token handling, query escaping, and shortcut metadata handling). This is a major-version-style update (PHP/dependency floor raises) and also introduces CI + static analysis to keep the new behavior covered.
Changes:
- Upgrade runtime/dependency floors (PHP
^8.2, Flysystem^3, Guzzle 7/psr7 v2) and add modern dev tooling (PHPUnit 10/11, PHPStan, Pint, GitHub Actions, Dependabot). - Security/correctness fixes in core adapter logic (trusted-host enforcement for streamed downloads, query-literal escaping, shortcut metadata handling, copy/move/delete edge cases).
- Introduce a credential-free Unit test suite plus reorganized Integration tests.
Reviewed changes
Copilot reviewed 24 out of 25 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/GoogleDriveAdapter.php |
Core adapter modernization plus security/correctness fixes (redirect handling, query escaping, shortcut metadata, delete/move/copy hardening). |
src/StreamableUpload.php |
Adds strict types and stronger typing/logic cleanup for resumable/multipart upload handling. |
composer.json |
Raises platform/dependency floors; adds dev tools and Composer scripts. |
phpunit.xml.dist |
Migrates to PHPUnit 10/11 config and splits Unit vs Integration suites. |
phpstan.neon |
Adds PHPStan config and targeted ignores for Google client magic properties. |
pint.json |
Adds Pint configuration for code style enforcement. |
.github/workflows/tests.yml |
Adds CI test matrix (PHP 8.2–8.4, prefer-lowest/prefer-stable). |
.github/workflows/static-analysis.yml |
Adds PHPStan workflow. |
.github/workflows/code-style.yml |
Adds Pint workflow. |
.github/dependabot.yml |
Enables Dependabot updates for Composer and GitHub Actions. |
tests/Unit/TrustedHostTest.php |
Regression tests for trusted-host check to prevent token leakage on redirects. |
tests/Unit/StaticHelpersTest.php |
Unit tests for query escaping, dirname normalization, and MIME guessing helpers. |
tests/Unit/ExposedGoogleDriveAdapter.php |
Test-only subclass exposing protected pure-logic helpers. |
tests/Unit/AdapterLogicTest.php |
Unit tests for adapter pure logic (path splitting, sanitization, param merging). |
tests/Integration/GoogleDriveAdapterTest.php |
Updates integration tests for new namespace/config and credential discovery. |
README.md |
Updates badges and documents version requirements/installation guidance. |
SECURITY.md |
Adds a security policy and disclosure guidance. |
CONTRIBUTING.md |
Adds contributor workflow, quality gates, and testing guidance. |
CHANGELOG.md |
Adds changelog documenting 3.0.0 breaking changes and fixes. |
.gitignore |
Updates ignores for new tooling caches and local environment files. |
.gitattributes |
Sets default LF handling and updates export-ignore list for dist packaging. |
.editorconfig |
Adds editor configuration for consistent formatting. |
.travis.yml |
Removes legacy CI config. |
.styleci.yml |
Removes legacy StyleCI config. |
.scrutinizer.yml |
Removes legacy Scrutinizer config. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- fetchResumeUri(): only send a content-length header when the request body size is known; getSize() may return null for streams of unknown size, which would otherwise produce an invalid header value - resume(): add the missing string parameter type to $resumeUri (getResumeUri() already promises a string) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| ## Installation | ||
|
|
||
| - For **Flysystem V2/V3** or **Laravel >= 9.x.x** | ||
| - For **PHP >= 8.2** / **Flysystem V3** / **Laravel 9.x – 13.x** |
There was a problem hiding this comment.
I don't understand this part.
This package doesn't use Laravel dependencies, so why put a limit?
I think that just Laravel >= 9.x.x more accurate.
That way it wouldn't be necessary to change it every time Laravel releases a new version.
Another question: does the current version not work on Laravel 13?
Because I see you set a limit of 12.x.
erikn69
reviewed
Jun 23, 2026
| | Package version | PHP | Flysystem | Laravel | | ||
| |-----------------|------------|-----------|----------------| | ||
| | `^3.0` | `>= 8.2` | `^3.0` | `9.x` – `13.x` | | ||
| | `^2.0` | `7.2 – 8.x`| `^2.1`/`^3.0` | `9.x` – `12.x` | |
Contributor
There was a problem hiding this comment.
does the current version not work on Laravel 13?
Yes, it works, Tested on 13.x, look at laravel-google-drive-ext-demo.
This package doesn't use Laravel dependencies, so why put a limit?
I think that just Laravel >= 9.x.x more accurate.
That's correct, it depends on Flysystem. Laravel handles compatibility, and as long as Flysystem doesn't release a major version, it will be compatible with future versions of Laravel.
Breaking changes (PHP/dependency floors), so this is really a 3.x-line
change
Updating dependency floors is not a breaking change; those that do not meet the requirements will simply not receive the changes, nothing will be broken.
Flysystem 2 was an incomplete release; it didn't last long and everyone immediately migrated to V3. It can be removed without any problems.
|
|
||
| on: | ||
| push: | ||
| branches: [ "2.x", "3.x", "master", "main" ] |
There was a problem hiding this comment.
"master", "main" ??
those branches do not exist
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.
Summary
This PR modernizes the package for current PHP/Flysystem and fixes several
correctness & security bugs surfaced by an adversarial code audit. It is
framework-agnostic — no Laravel/Illuminate coupling is introduced.
Security fixes
readStream()token leakage: the OAuth bearer token (and cookies) were sentto whatever host a download redirect pointed to. They're now only sent to
verified Google-owned hosts, over a connection with TLS peer verification, with
the socket connection guarded. Includes unit regression tests for the host check
(suffix/prefix spoofing, lookalikes).
(
name = '…') are now escaped.Correctness fixes
(
shortcutDetailswas never requested by the GET fetch); now requested + guarded.copy()could callgetRawVisibility()onnullwhen the source was resolvedpurely from the path cache.
delete()swallowed all throwables and reported success on real failures; it nowonly treats a missing path as a no-op success and surfaces genuine API errors.
move()could report false success when the post-update fetch returned no file.for-loop condition in the path-cache builder (comparedinttoarray); hardened severalnull/falsecases;writeData()now forwards theunderlying error as the exception cause.
Modernization
^8.2; drops EOL Guzzle 6 / psr7 v1; requiresleague/flysystem ^3.0,guzzlehttp/guzzle ^7.5,guzzlehttp/psr7 ^2.4.(PHP 8.2/8.3/8.4 × prefer-lowest/prefer-stable), PHPStan (level 5), and
Pint. Added Dependabot.
UnitandIntegrationsuites; added a credential-free unit test suite so the corelogic is finally covered in CI (the existing integration tests still skip without
credentials).
define('DEBUG_ME')debug scaffolding; added native parameterand return types throughout; fixed typos and stale comments.
CHANGELOG.md,CONTRIBUTING.md,SECURITY.md; refreshed README badgesand added a requirements matrix.
Verification
composer validate, PHPStan, Pint, and the unit suite all pass locally on PHP 8.5;the integration suite is skipped (no credentials), as before.
I'm happy to split this into smaller PRs (e.g. CI-only, then fixes) if that's
easier to review.