security: Request::getFile() empty-tmp_name guard + RedirectGuard open-redirect allowlist (#408)#414
Merged
Conversation
…n-redirect allowlist (#408) 評価レポート (#401 § 4 Security) 指摘の 2 件を解消。 ### Request::getFile() 強化 \`\$_FILES[\$key]\` の structure validation を前段で追加 (defense in depth): 1. canonical 5 keys (name/type/tmp_name/error/size) すべて存在を要求 → 任意 key 欠落で null 2. \`error === UPLOAD_ERR_NO_FILE\` の場合は null → 「ファイル未選択」の semantic を getFile レベルで吸収 UPLOAD_ERR_NO_FILE 以外の error (INI_SIZE, PARTIAL, NO_TMP_DIR 等) は 従来通り UploadedFile を生成 → controller は validate() 経由で error コードを surface できる (情報を消さない)。 ### Nene\Xion\RedirectGuard (新規) \`ControllerBase::location(\$uri, false)\` の open-redirect surface を fail-closed allowlist で守る: - \`isExternalAllowed(string \$uri): bool\` (predicate、test 容易) - \`ensureExternalAllowed(string \$uri): void\` (throw on deny) - \`allowedHosts(): array\` (env 解析公開) - Default policy: env (NENE_ALLOWED_EXTERNAL_REDIRECTS) 空 or 未設定で 全 external host を拒否 → controller が user-supplied URL を素通し しても phishing 誘導にならない - comma-separated host allowlist、大文字小文字を ignore、trim する ControllerBase::location() を \$flag=false 経路で \`RedirectGuard::ensureExternalAllowed()\` 呼出しに切替。\$flag=true 経路 (internal redirect under URI_ROOT) は不変。 ### Tests - tests/Unit/Xion/RequestGetFileGuardTest.php (6 cases): 4 deny + 2 pass - tests/Unit/Xion/RedirectGuardTest.php (8 cases): allowlist / deny / case-insensitive / no-host / throw / trim semantics ### Compose / env / docs - compose.yaml に NENE_ALLOWED_EXTERNAL_REDIRECTS (default 空) - docs/development/production-deployment.md env matrix に 1 行追加 (fail-closed の挙動 + 推奨 value pattern を明記) ### Verification - composer test 113/113 (99 既存 + 6 getFile + 8 RedirectGuard) - composer test:http 24/24 (1 expected skip) - composer analyze (Phan) exit 0 - composer format:check exit 0 Closes #408. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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.
評価レポート #401 § 4 Security の 2 件を fail-closed で対応。
Request::getFile() defense-in-depth
`$_FILES[$key]` の structure を前段で validate:
UPLOAD_ERR_NO_FILE 以外の error は従来通り UploadedFile 生成 → controller が validate() 経由で error code を surface 可能 (情報非破壊)。
`Nene\Xion\RedirectGuard` (新規)
`ControllerBase::location($uri, false)` の open-redirect surface を fail-closed allowlist で守る:
`location()` の $flag=true 経路 (internal redirect) は不変、$flag=false 経路のみ guard 経由に。
Tests
Compose / env
`NENE_ALLOWED_EXTERNAL_REDIRECTS` (default 空) + production-deployment.md matrix 1 行。
Test plan
Closes #408.