kaxil opened a new pull request, #73534: URL: https://github.com/apache/airflow/pull/73534
The Modal sandbox backend offered two egress modes. `block_network=True` is exact. A hostname allowlist is matched on the TLS handshake name, and it is refused unless the author opts in with `egress_enforcement="sni"`, because it is weak in ways the spec does not show: TLS on 443 only, destination address ignored, name resolution open for every host. That left "deny everything or accept the caveats" as the only choice, when Modal has a third mode the backend never exposed. `outbound_cidr_allowlist` is enforced on the destination address for any port and protocol and denies by default outside the list. This adds `SandboxSpec.allow_egress_to_cidrs`. On Modal it maps onto that allowlist and needs no opt-in, since there is no caveat for the author to accept. It is the mode for the case the hostname list serves worst and a platform team most often means by restricted egress: this sandbox may reach one internal service at a fixed address and nothing else. It cannot serve a package registry behind a CDN, whose addresses rotate faster than a sandbox lives, so the hostname list stays for that job. `sbx` refuses the field; its per-sandbox rule takes hostnames and there is nothing to map a range onto. **Measured on a live sandbox before writing the docs**, because the open question on this design was whether name resolution survives an address-only list. With `["1.1.1.1/32"]`: the listed address connected on 443 and on 53, an unlisted address timed out on both, and `pypi.org` still resolved through Modal's own resolver inside the sandbox. So hostnames resolve and then their addresses are unreachable. The tool description says exactly that, so the model does not read a successful lookup as a reachable host. It also means DNS is still a channel out under this mode, as under the hostname list; only `block_network=True` with no allowlist closes it, and the docs say so. **Entries are validated and normalised**, since Modal passes the list through unchecked. A hostname, a URL or a `host:port` would be accepted by Modal and match nothing while reading as a restriction, so they are refused. A bare address is written as `/32` (`/128` for IPv6). A range with host bits set, such as `10.0.0.1/8`, is refused rather than widened to `10.0.0.0/8`, because that is not what the author wrote. `0.0.0.0/0` and `::/0` are refused as well: an allowlist of every address is an open network wearing a restriction's clothes, and `block_network=False` is the honest way to ask for one. **The two lists combine, and combining weakens the address one.** Modal applies them additively, and I measured what that means: adding `pypi.org` to the hostname list beside `["1.1.1.1/32"]` made a TCP connection to `8.8.8.8:443` succeed, because port 443 is then routed by handshake name for every address. A combined spec therefore has the address list's guarantee on every port except 443 and the hostname list's caveats there. The hostname half keeps its opt-in when combined, so adding an address list cannot launder the hostname list past it. **Why a spec field rather than a backend parameter.** The tool description reads the spec to tell the model what it can reach, and a network policy split across two objects is one the description would get wrong. A spec field also follows the contract's existing rule: a backend enforces it or refuses it, so a Dag moved to `sbx` fails loudly instead of running with an open network the author believed was closed. The Modal system test gains a second task that checks the three measured facts on a real sandbox. No behaviour changes for specs that do not set the new field: the default, `block_network=False`, and the hostname list under both enforcement settings map exactly as before, and the existing tests for each still pass unchanged. --- * Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information. Note: commit author/co-author name and email in commits become permanently public when merged. * For fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed. * When adding dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x). * For significant user-facing changes create newsfragment: `{pr_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments). You can add this file in a follow-up commit after the PR is created so you know the PR number. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
