Feat: Support trusted console projects#11248
Conversation
📝 WalkthroughWalkthroughAdds Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
Security Scan Results for PRDocker Image Scan Results
Source Code Scan Results🎉 No vulnerabilities found! |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @.env:
- Line 28: Move the _APP_CONSOLE_TRUSTED_PROJECTS key so it appears with the
other console-related environment variables to satisfy dotenv ordering rules;
locate the line containing
_APP_CONSOLE_TRUSTED_PROJECTS=trusted-project,another-trusted-project and
cut/paste it into the block where other console keys (e.g., keys prefixed with
_APP_CONSOLE_ or nearby console-related entries) are defined, preserving its
value and formatting.
In `@app/init/resources.php`:
- Around line 259-270: The trusted-projects parsing silently fails on entries
with whitespace; update the block that builds $trustedProjects (currently
looping over explode(',', System::getEnv('_APP_CONSOLE_TRUSTED_PROJECTS', '')))
to trim each exploded value and remove empty strings before matching against
$rule->getAttribute('projectId', ''), e.g. use array_map('trim', ...) and
array_filter(...) to produce $trustedProjects and then use in_array to set
$permitsCurrentProject; keep the existing condition that checks
!$permitsCurrentProject and !$rule->isEmpty() and the final in_array check.
In `@tests/e2e/Services/Projects/ProjectsConsoleClientTest.php`:
- Around line 5176-5181: The test testConsoleCorsWithTrustedProject currently
hard-codes $trustedProjectIds; instead, read the runtime env variable
_APP_CONSOLE_TRUSTED_PROJECTS, split it on commas, trim and filter empty values
into $trustedProjectIds, and then merge with the untrusted ID as before (use the
existing $projectIds = array_merge($trustedProjectIds,
['untrusted-project-id'])); ensure the parsing handles empty or missing env
gracefully (fall back to an empty array).
- Around line 5182-5234: The finally block unconditionally attempts to delete
the project and asserts a 204, which can mask a prior failure in setupProject;
add a guard variable (e.g., $created = false) before calling setupProject(), set
$created = true only after setupProject() succeeds (or after receiving a
successful creation response), and in the finally block only call the delete via
$this->client->call(... '/projects/' . $projectId ...) and assert the 204 when
$created is true; reference setupProject, the foreach over $projectIds, and the
delete call to locate where to add the flag and conditional cleanup.
There was a problem hiding this comment.
Pull request overview
Adds support for “trusted” projects whose custom-domain rules can be used to allow CORS access to Console APIs (and potentially other projects), driven by a new _APP_CONSOLE_TRUSTED_PROJECTS env var.
Changes:
- Introduces
_APP_CONSOLE_TRUSTED_PROJECTSto control which project IDs are treated as trusted. - Updates rule resolution to allow trusted projects’ rules to be accepted even when the rule’s owning project doesn’t match the current project.
- Adds an E2E test covering Console CORS behavior for trusted vs untrusted projects.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
app/init/resources.php |
Adds trusted-project bypass logic when resolving the request origin “rule”. |
tests/e2e/Services/Projects/ProjectsConsoleClientTest.php |
Adds an E2E test to validate Console CORS behavior for trusted/untrusted projects. |
docker-compose.yml |
Plumbs _APP_CONSOLE_TRUSTED_PROJECTS into relevant containers. |
.env |
Adds a default value for _APP_CONSOLE_TRUSTED_PROJECTS. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
✨ Benchmark results
⚡ Benchmark Comparison
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
What does this PR do?
Allows to trust specific projects so all it's rules is allowed to talk to all projects including Console.
Test Plan
New tests
Related PRs and Issues
x
Checklist