fix(server): enforce renew expiresAt must be after current expiresAt#639
fix(server): enforce renew expiresAt must be after current expiresAt#639ZYecho11 wants to merge 1 commit intoalibaba:mainfrom
Conversation
Add the missing validation to both Docker and Kubernetes renew_expiration implementations. Return HTTP 400 with INVALID_EXPIRATION error code when the requested time is not strictly after the current expiration.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee7f9847dc
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ), | ||
| }, | ||
| ) | ||
| if new_expiration <= current_expiration: |
There was a problem hiding this comment.
Preserve renew-intent no-op behavior for equal expirations
The new strict check rejects new_expiration == current_expiration, but the renew-intent controller intentionally computes new_expires = max(now + extend, current) (see server/opensandbox_server/integrations/renew_intent/controller.py), so while a sandbox still has more than extend seconds left this path submits an equal timestamp. After this change those internal renew attempts now fail with 400, and in proxy-only mode that means last_success_monotonic is never updated (consumer.py cooldown is success-based), which can trigger repeated renew calls and warning logs on every access until close to expiry. Either adjust renew-intent to request a strictly later time or keep equality idempotent for this internal flow.
Useful? React with 👍 / 👎.
|
|
Add the missing validation to both Docker and Kubernetes renew_expiration implementations. Return HTTP 400 with INVALID_EXPIRATION error code when the requested time is not strictly after the current expiration.
Summary
Testing
Breaking Changes
Checklist