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 3 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 |
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
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".
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
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".
| for _, sbx := range orphanSandboxes { | ||
| wg.Go(func() { | ||
| ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), sbxRemoveTimeout) | ||
| defer cancel() | ||
| s.callbacks.RemoveSandboxFromNode(ctx, sbx) |
There was a problem hiding this comment.
Store.Reconcile now unconditionally treats every sandbox returned by storage.Reconcile as an orphan and calls RemoveSandboxFromNode, but the memory/populate-redis path still returns sandboxes that are missing from cache and should be added (packages/api/internal/sandbox/storage/memory/sync.go:53-70, forwarded by packages/api/internal/sandbox/storage/populate_redis/main.go:88-89). Since node sync always invokes store.Reconcile (packages/api/internal/orchestrator/nodemanager/sync.go:55) and both config and IaC defaults still use the memory backend (packages/api/internal/cfg/model.go:88, iac/modules/job-api/variables.tf:107-110), an API restart or cache desync will cause healthy running sandboxes to be killed instead of re-registered.
Useful? React with 👍 / 👎.
|
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 |
|
3 participants
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: