Repository navigation
fix(deps): bump modal from 1.4.3 to 1.6.1 - #5400
dependabot[bot] wants to merge 3 commits into
Conversation
Bumps [modal](https://github.com/modal-labs/modal-client) from 1.4.3 to 1.6.1. - [Changelog](https://github.com/modal-labs/modal-client/blob/main/CHANGELOG_DEV.md) - [Commits](modal-labs/modal-client@py/v1.4.3...py/v1.6.1) --- updated-dependencies: - dependency-name: modal dependency-version: 1.6.1 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsAdvisory findings (1)ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Fixed the Modal 1.6.1 compatibility failure: Added executable regression coverage for binary prefixes, shell-sensitive filenames, missing files, directories, and FIFOs. Replaced tests of the removed provider descriptor API with tests of the supported execution boundary. Validation: targeted mypy passed; 188 original focused tests passed, then 72 bounded-read tests passed after the additional unsupported-file coverage. Three adversarial review rounds completed, with the final two consecutive rounds clean. Full local verification and CI results follow below. Local validation on byte-identical source exported to a neutral path (the existing MCP tests treat the literal |
![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: 28d8b29875
ℹ️ 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".
markstuart-oai
left a comment
markstuart-oai
left a comment
There was a problem hiding this comment.
Reviewed all four changed files at 28d8b29875e58821091625246abe6228eab29386 and the supporting sandbox contracts statically. The Modal transport replacement preserves bounded binary reads and path argument separation, but the new 30-second deadline can terminate the whole sandbox and unrelated work. I agree with the existing bounded-read timeout finding; no duplicate inline comment.
This was a source review. I did not run candidate code or tests. Current hosted CI: 4 skipped, 23 success.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a197bceed5
ℹ️ 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".
| raise | ||
| # Bound acquisition inside the sandbox, using the same exec transport as read(). | ||
| # Like read(), do not impose an exec deadline: it terminates the whole sandbox. | ||
| result = await self.exec( |
There was a problem hiding this comment.
When cancellation arrives after Modal has created the head process but while stdout is still being read—for example, when a regular file on a slow mounted filesystem blocks—_exec_internal() propagates CancelledError without calling ContainerProcess.terminate(). The remote process therefore survives the cancelled read_bounded() call, and repeated cancellations can accumulate blocked processes until the sandbox expires; retain the process handle and terminate it during cancellation cleanup without terminating the entire sandbox.
AGENTS.md reference: AGENTS.md:L117-L118
Useful? React with 👍 / 👎.
| ] | ||
| modal = [ | ||
| "modal==1.4.3", | ||
| "modal==1.6.1", |
There was a problem hiding this comment.
When workspace_persistence is snapshot_filesystem or snapshot_directory, this upgrade crosses Modal 1.5, which changed both snapshot APIs from indefinite retention to a 30-day default. The persistence methods still call these APIs without ttl= and serialize only the resulting image ID, so after 30 days a previously durable workspace can no longer be restored. Pass ttl=None explicitly to preserve the released behavior; Modal documents this breaking default in its Python SDK release notes.
AGENTS.md reference: AGENTS.md:L111-L111
Useful? React with 👍 / 👎.
| ] | ||
| modal = [ | ||
| "modal==1.4.3", | ||
| "modal==1.6.1", |
There was a problem hiding this comment.
When a directory snapshot takes more than 55 seconds, Modal 1.5 now terminates it at a new provider-side default even if snapshot_filesystem_timeout_s is None or greater than 55. _persist_workspace_via_snapshot_directory() only applies that public setting through an outer asyncio.wait_for() and invokes snapshot_directory.aio(root) without timeout=, so the configured 60-second default and larger user values no longer control the operation. Forward the configured timeout into the Modal call; the new upstream deadline is documented in the Modal 1.5 release notes.
AGENTS.md reference: AGENTS.md:L115-L115
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
Here are some automated security review suggestions for this pull request.
Reviewed commit: a197bceed5
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
| "/bin/sh", | ||
| "-c", | ||
| "PATH=/usr/bin:/bin; export PATH; " | ||
| '[ -e "$1" ] || exit 44; [ -f "$1" ] || exit 45; head -c "$2" < "$1"', |
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
When a Modal-backed agent can modify image utilities, model-controlled shell commands can replace /usr/bin/head before the host or a sink calls read_bounded(). This line makes that mutable binary the only enforcement of max_bytes; _exec_internal() then drains stdout without a byte limit or timeout, so a replacement can emit arbitrary output and exhaust or indefinitely stall the host. Fixed PATH and quoted arguments prevent injection, but not binary tampering. Enforce the limit in the trusted provider/host transport.
SECURITY.md reference: SECURITY.md:L38-L38
Dismiss this finding: Reply with @codex security dismiss <reason> [context]. Codex will resolve this conversation automatically; GitHub may require a page refresh to show the result.
Valid reasons: false-positive, duplicate, out-of-scope, compensating-control, risk-accepted, or other. Example: @codex security dismiss duplicate Already flagged by another review
What each reason means
false-positive— Not a vulnerabilityduplicate— Already tracked elsewhereout-of-scope— Outside this review's scopecompensating-control— Mitigated by another controlrisk-accepted— Risk intentionally acceptedother— Another reason; context required
Useful? React with 👍 / 👎.
|
Closing this update after review established that the exec-based bounded-read adaptation is not safe to merge. The repository's tested Modal lock stays on 1.4.3. Unresolved blockers:
The timeout issue already addressed in a197bce does not resolve these remaining problems. They remain unresolved rather than being dismissed. A follow-up provider migration should establish a trusted bounded/cancellable read mechanism and cover both snapshot compatibility changes before retrying the dependency upgrade. Upstream migration/default changes: https://modal.com/docs/sdk/py/releases . The filesystem and process API limitations were also verified against the installed 1.6.1 implementation. |
|
OK, I won't notify you again about this release, but will get in touch when a new version is available. If you'd rather skip all updates until the next major or minor version, let me know by commenting If you change your mind, just re-open this PR and I'll resolve any conflicts on it. |
Bumps modal from 1.4.3 to 1.6.1.
Commits
2bc73c2Release v1.6.1 of the Python SDK (#64216)d5a32e6client: [python] Send the sandbox token from create on later Sandbox RPCs (#6...12c3933Add sandbox_token to SandboxCreateV2Response (#64200)c943992Unlimited timeout for session start and terminate (#63593)b734c71Remove "(label truncated)" annotation from web URL output in modal deploy (#6...04a7bdf[client] bump s11y to v0.12.6 (#64090)1e3c889[SDK] Include AppHandleMetadata in AppList RPC (#63547)057ad74[CLI/SDK] Redefine total container count in function/server/endpoint stats (#...08676bbClarify that Volume.commit() and reload() run inside a container (#60053)8d5894dRevise modal skill for better coding agent dev ex (#60060)Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)