docs: add sandboxing API proposal#1171
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a design proposal for a pluggable sandboxing API, a typed Python API contract module with sandbox lifecycle and execution hooks, and updates the spelling wordlist with sandboxing-related terms. Estimated code review effort: 2 (Simple) | ~12 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@pavank63 Here is is my proposal. I have listed you as a co-author. Part of the document are based on your previous proposal and research. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/proposals/sandboxing_api.py (1)
12-73: ⚡ Quick winAdd Sphinx version directives to the new public hook docstrings.
These are user-facing API additions, so include
.. versionadded::(or.. versionchanged::where applicable) in each public function docstring.Suggested pattern
def setup_sandbox( @@ ) -> None: """Set up sandboxing + + .. versionadded:: <release> Executed after `prepare_source` hook. Can be used to set up sandbox or chown/chmod the sdist_root_dir. """As per coding guidelines
**/*.{rst,py}: Use Sphinx versionadded, versionremoved, versionchanged directives for user-facing changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/proposals/sandboxing_api.py` around lines 12 - 73, Add Sphinx version directive lines to each public hook docstring: update setup_sandbox, teardown_sandbox, run_sandboxed, and run_unconfined to include a ".. versionadded:: <release>" (or ".. versionchanged:: <release>" if appropriate) entry immediately after the short description/summary in each docstring; use the correct release version token for this change and keep the directive formatting consistent with other public API docstrings in the repo.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/proposals/sandboxing_api.py`:
- Line 47: The cwd parameter in run_sandboxed and run_unconfined is currently
typed as os.PathLike[str] | os.PathLike[bytes] | None which excludes plain str
and bytes accepted by subprocess.run; update the type annotation for cwd in both
functions to include str and bytes (e.g., os.PathLike[str] | os.PathLike[bytes]
| str | bytes | None) so it matches subprocess.run’s signature and adjust any
imports/typing as needed.
---
Nitpick comments:
In `@docs/proposals/sandboxing_api.py`:
- Around line 12-73: Add Sphinx version directive lines to each public hook
docstring: update setup_sandbox, teardown_sandbox, run_sandboxed, and
run_unconfined to include a ".. versionadded:: <release>" (or "..
versionchanged:: <release>" if appropriate) entry immediately after the short
description/summary in each docstring; use the correct release version token for
this change and keep the directive formatting consistent with other public API
docstrings in the repo.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fc037f41-cfbf-4ae6-a79b-9cba905dc67b
📒 Files selected for processing (3)
docs/proposals/sandboxing-api.mddocs/proposals/sandboxing_api.pydocs/spelling_wordlist.txt
tiran
force-pushed
the
proposal-sandbox-api
branch
from
cf3fcdb to
159b672
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/proposals/sandboxing-api.md`:
- Around line 58-60: The proposal currently contradicts itself about the
run_unconfined hook by describing it for monitoring/policy but only adding
WorkContext.run_unconfined and omitting BuildEnvironment.run_unconfined; clarify
the API contract by either (A) adding run_unconfined to BuildEnvironment as well
(or documenting a clear delegation from BuildEnvironment to WorkContext) so
build-step code has a defined call path, or (B) explicitly state that unconfined
execution is only permitted via WorkContext.run_unconfined (and remove any
mentions implying BuildEnvironment exposure); update the rationale text and any
instances around the run_unconfined mentions (including the other affected
paragraphs) to match the chosen design and ensure there is a single unambiguous
call-site for unconfined execution.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 45703514-441f-438b-8c55-b868dd5f9d7e
📒 Files selected for processing (3)
docs/proposals/sandboxing-api.mddocs/proposals/sandboxing_api.pydocs/spelling_wordlist.txt
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/proposals/sandboxing_api.py
|
This pull request has merge conflicts that must be resolved before it can be merged. |
tiran
force-pushed
the
proposal-sandbox-api
branch
from
159b672 to
d6d1171
Compare
tiran
force-pushed
the
proposal-sandbox-api
branch
from
d6d1171 to
470602d
Compare
|
The proposal looks good to me overall. One comment: could we clarify whether |
tiran
force-pushed
the
proposal-sandbox-api
branch
from
470602d to
c096dd9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/proposals/sandboxing-api.md`:
- Around line 159-165: The Landlock row in the sandboxing capability table has
an incorrect IPC-scoping version. Update the `Landlock` entry in the table to
reflect that `LANDLOCK_SCOPE_SIGNAL` and related socket-scoping support are
available starting in Linux 6.12 / Landlock ABI 6, and change the note from
`6.11+` to `6.12+`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5e7404e1-c971-4b43-ade1-c72d3ba447d8
📒 Files selected for processing (3)
docs/proposals/sandboxing-api.mddocs/proposals/sandboxing_api.pydocs/spelling_wordlist.txt
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/proposals/sandboxing_api.py
Proposal for a pluggable sandboxing API that lets users configure custom sandboxing solutions for build process confinement. Co-Authored-By: Pavan Kalyan Reddy Cherupally <pcherupa@redhat.com> Co-Authored-By: Claude <claude@anthropic.com> Signed-off-by: Christian Heimes <cheimes@redhat.com>
tiran
force-pushed
the
proposal-sandbox-api
branch
from
c096dd9 to
1421a8b
Compare
2 participants
Pull Request Description
What
Proposal for a pluggable sandboxing API that lets users configure custom sandboxing solutions for build process confinement.
Why
#1019