fix(snapshot): scope GetLastSnapshot query by teamID to prevent unauthorized cross-team access#2638
fix(snapshot): scope GetLastSnapshot query by teamID to prevent unauthorized cross-team access#2638AdaAibaby wants to merge 5 commits into
Conversation
|
Atlantis commands can't be run on fork pull requests. To enable, set --allow-fork-prs or, to disable this message, set --silence-fork-pr-errors |
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8e62f63f0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Code Review
This pull request introduces a GetByTeam method to the SnapshotCache to enforce team-level scoping for snapshot lookups at the database layer, replacing manual ownership checks in various API handlers. It includes corresponding database queries, comprehensive unit tests, and updates to the OpenAPI specification to support sandbox creation from snapshots. A syntax error was identified in the new test file where the package declaration is duplicated.
…napshotID from spec - Remove duplicate 'package snapshotcache' line in snapshot_cache_test.go (caused compile error, caught by Codex/Gemini review) - Revert snapshotID field from NewSandbox schema and related descriptions in openapi.yml — backend handler not yet implemented, advertising it breaks the API contract (caught by Codex P2 review)
|
Atlantis commands can't be run on fork pull requests. To enable, set --allow-fork-prs or, to disable this message, set --silence-fork-pr-errors |
|
|
Atlantis commands can't be run on fork pull requests. To enable, set --allow-fork-prs or, to disable this message, set --silence-fork-pr-errors |
|
|
Atlantis commands can't be run on fork pull requests. To enable, set --allow-fork-prs or, to disable this message, set --silence-fork-pr-errors |
|
|
Atlantis commands can't be run on fork pull requests. To enable, set --allow-fork-prs or, to disable this message, set --silence-fork-pr-errors |
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f117e07a5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Atlantis commands can't be run on fork pull requests. To enable, set --allow-fork-prs or, to disable this message, set --silence-fork-pr-errors |
|
|
Atlantis commands can't be run on fork pull requests. To enable, set --allow-fork-prs or, to disable this message, set --silence-fork-pr-errors |
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b9f5f94ed
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Atlantis commands can't be run on fork pull requests. To enable, set --allow-fork-prs or, to disable this message, set --silence-fork-pr-errors |
|
|
This isn't really possible as we need to access the snapshot in some places without knowing team ID apriori and we don't want to maintain 2 methods doing the same. Please propose the solution in the issue before starting the implementation next time 🙏🏻 |
Thanks a lot for the explanation, Jakub — that makes perfect sense. |
Fixes #2637
Problem
Four handlers (
sandbox_connect,sandbox_get,sandbox_pause,sandbox_resume) calledSnapshotCache.Get(ctx, sandboxID)which fetches a snapshot by sandbox ID only, thenperformed a post-fetch ownership check: