docs: document safer execution patterns#1393
Conversation
Signed-off-by: Akihiko Kuroda <akihikokuroda2020@gmail.com>
github-actions
Bot
added
the
documentation
| policy=CapabilityPolicy( | ||
| timeout=30, # Kill after 30 seconds | ||
| network_access=False, # No network access | ||
| package_installation=False, # No pip install | ||
| ), |
There was a problem hiding this comment.
I looked at the docstring for CapabilityPolicy; I see:
The enforcement gap — the difference between what is *declared* and what is
*actively enforced at runtime* — is made explicit through per-field
`ENFORCED_*` class attributes. Callers and UX layers can read these to
decide whether to prompt the user ("allow once / allow always") or display
a warning.
.. warning::
**Most fields are declarative only and are not enforced at runtime.**
Setting `network_access=False`, `package_installation=False`,
`subprocess_execution=False`, or `env_var_access=False` does *not*
block those capabilities in the current implementation — the declared
value is informational only. Check the `ENFORCED_*` class attributes
to see what is actively enforced. For stronger isolation use a
`"docker"` execution tier.
`filesystem_read_roots` and `filesystem_write_roots` emit a
:class:`UserWarning` at construction time when set to a non-`None`
value, because those restrictions are also unenforced.
If this is true and these things aren't enforced, then I don't think we should advertise them as such in these examples / code snippets. If these things are enforced, please change the CapabilityPolicy docstring.
There was a problem hiding this comment.
Code snippet is updated.
- Added a clarifying comment explaining that most policy fields are declared only, not enforced
- Updated inline comments to explicitly distinguish between enforced fields (timeout, max_output_chars) and declared-only fields (network_access, package_installation)
- Referenced the escape hatch (CapabilityPolicy.ENFORCED_* class attributes) so readers know how to check what's enforced
There was a problem hiding this comment.
Can you please explain how this works in general then? How does a CapabilityPolicy actually impact the execution? And what is the point to having these "declarative" fields that aren't enforced? It seems misleading to me.
There was a problem hiding this comment.
Took out these 2 policy setting.
Signed-off-by: Akihiko Kuroda <akihikokuroda2020@gmail.com>
| |------|-----------|-------|----------| | ||
| | `"static"` | Analysis only (no execution) | Fastest | Fast validation before execution | | ||
| | `"local_unsafe"` | None (direct subprocess) | Fast | Trusted code only, max performance | | ||
| | `"local"` | None (policy control only) | Fast | Trusted code with resource limits (timeout, memory) | |
There was a problem hiding this comment.
Likely should be (timeout, sdtoutput limits) - I don't think we restrict memory anywhere via policy even in the docker tier.
There was a problem hiding this comment.
Thanks! it is corrected.
Signed-off-by: Akihiko Kuroda <akihikokuroda2020@gmail.com>
markstur
left a comment
markstur
left a comment
There was a problem hiding this comment.
a couple inline wording nits. If you disagree, I don't mind if you ignore these nits.
Signed-off-by: Akihiko Kuroda <akihikokuroda2020@gmail.com>
Signed-off-by: Akihiko Kuroda <akihikokuroda2020@gmail.com>
My nits were resolved. Thanks! |
4 participants
Pull Request
Issue
Fix: #1310
Description
Testing
Attribution
Adding a new component, requirement, sampling strategy, or tool?
If your PR adds or modifies one of the types below, check the matching box. A checklist of type-specific review items will be posted as a comment.
NOTE: Please ensure you have an issue that has been acknowledged by a core contributor and routed you to open a pull request against this repository. Otherwise, please open an issue before continuing with this pull request.