Skip to content

feat(sandbox): add plugin execution sandbox with resource limits and …#367

Closed
ask-z4ch wants to merge 6 commits into
utksh1:mainfrom
ask-z4ch:main
Closed

feat(sandbox): add plugin execution sandbox with resource limits and …#367
ask-z4ch wants to merge 6 commits into
utksh1:mainfrom
ask-z4ch:main

Conversation

@ask-z4ch
Copy link
Copy Markdown
Contributor

@ask-z4ch ask-z4ch commented May 28, 2026

Purpose & Rationale

Plugin subprocesses (nmap, nikto, custom parsers) previously ran with no CPU, memory, or time constraints. A single runaway plugin could degrade the entire local instance — common in the learning and lab environments SecuScan targets. This change introduces a backend sandbox layer that enforces configurable resource limits with structured timeout escalation, surfacing violation reasons in the Task Details view.

Technical Resolution Details

Added backend/secuscan/sandbox_executor.py with sandbox_execute() wrapping every asyncio.create_subprocess_exec call with timeout enforcement (asyncio.wait_for), output byte-stream capping, resource.setrlimit via preexec_fn on Linux (with platform.system() guard for macOS/Windows), and a SIGTERM → 3s grace → SIGKILL escalation. Created SandboxConfig Pydantic model (timeout_seconds=120, max_memory_mb=512, max_output_bytes=5MB, allow_network) and SandboxViolation model in models.py. Extended PluginMetadata with an optional sandbox key for per-plugin overrides. Replaced raw subprocess calls in executor.py (_execute_command) with sandbox_execute(), mapping SandboxViolation reasons to distinct terminated:timeout, terminated:memory_limit, and terminated:output_limit task statuses. Updated frontend/src/pages/TaskDetails.tsx with a labeled badge, threshold tooltip, and sandbox violation error block. Documented the sandbox metadata key in PLUGINS.md. Closes #326.

Local Testing Execution

  1. pytest testing/backend/unit/ testing/backend/test_sandbox_executor.py -v --asyncio-mode=strict --timeout=60 — 254 tests pass; 2 failures and 2 errors are pre-existing infrastructure issues (missing trio dependency, Windows PermissionError on temp dir cleanup) unrelated to this change
  2. All 11 new sandbox-specific tests pass: timeout path, output cap, SIGKILL escalation, truncation flagging, broadcast callback, platform guard, and resolve_sandbox_config merging
  3. Manual verification confirms SandboxViolation(reason="timeout") produced for a sleep-30 process with 1s timeout, and SandboxViolation(reason="output_limit") for a 5000-byte output capped at 100 bytes

Desired Review Feedback Type

Focus on the sandbox_execute SIGTERM → SIGKILL escalation timing and the output-cap truncation logic — these paths are the most safety-critical.

Integrity & AI Usage Disclosure

In compliance with the GSSoC 2026 AI Conduct rules, I disclose that AI tools were used solely as learning and boilerplate aids. The final logic was fully reviewed, tested, and manually adapted to match human styling and clean-code design.

ask-z4ch added 2 commits May 28, 2026 10:51
@ask-z4ch
feat(sandbox): add plugin execution sandbox with resource limits and …
06d5e05 
…timeout enforcement

Closes utksh1#326

Introduce SandboxConfig Pydantic model, sandbox_executor.py utility,
and frontend Task Details integration for resource-limited subprocess
execution.

- SandboxConfig with defaults (timeout_seconds=120, max_memory_mb=512,
  max_output_bytes=5MB, allow_network)
- sandbox_execute() wraps asyncio.create_subprocess_exec with:
  - asyncio.wait_for() timeout enforcement
  - stdout/stderr byte-stream capping at max_output_bytes
  - resource.setrlimit (RLIMIT_AS, RLIMIT_CPU) via preexec_fn on Linux
  - SIGTERM -> 3s grace -> SIGKILL escalation
  - Structured SandboxViolation propagated to task status
- Per-plugin sandbox overrides via metadata.json sandbox key
- Task-status values: terminated:timeout, terminated:memory_limit,
  terminated:output_limit
- Frontend Task Details: labeled badge with threshold tooltip,
  sandbox violation error block
- 11 new unit tests covering timeout, output cap, SIGKILL, platform guard
- Documented sandbox metadata key in PLUGINS.md

Signed-off-by: ask-z4ch <askz4ch@users.noreply.github.com>
@ask-z4ch
fix(test): update _execute_command mocks to return 3-tuple
fecfd42 
The sandbox executor refactored _execute_command to return
(output, exit_code, violation) instead of (output, exit_code).
All mocked call sites in integration tests must return None
as the third element to match the new signature.

Closes utksh1#326

Signed-off-by: ask-z4ch <askz4ch@users.noreply.github.com>
@utksh1 utksh1 added area:backend Backend API, database, or service work area:plugins Scanner plugin metadata, schemas, or plugin runtime work area:security Security-sensitive implementation or tests type:security Security work category bonus label type:feature Feature work category bonus label type:testing Testing work category bonus label level:critical 80 pts difficulty label for critical or high-impact PRs labels May 28, 2026
utksh1
utksh1 requested changes May 28, 2026
View reviewed changes
Copy link
Copy Markdown
Owner

@utksh1 utksh1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes. The sandbox direction is useful, but this is not safe enough to merge yet: memory-limit termination is not reliably classified as terminated:memory_limit, the old timeout argument path is effectively replaced instead of preserving compatibility, and output-limit handling can still broadcast/read chunks beyond the capped stored output. Please make the limit behavior precise and add tests for timeout, memory, output, cancellation, and legacy timeout compatibility.

@utksh1
Copy link
Copy Markdown
Owner

utksh1 commented May 28, 2026

Thanks for following up. Clarifying the change request so it is actionable:

Why this is blocked:
Requesting changes. The sandbox direction is useful, but this is not safe enough to merge yet: memory-limit termination is not reliably classified as terminated:memory_limit, the old timeout argument path is effectively replaced instead of preserving compatibility, and output-limit handling can still broadcast/read chunks beyond the capped stored output. Please make the limit behavior precise and add tests for timeout, memory, output, cancellation, and legacy timeout compatibility.

What to do next:

  • Fix the specific issues called out above.
  • Push the updated branch and make sure the relevant CI checks pass.
  • Reply here when ready for re-review.

ask-z4ch added 4 commits May 29, 2026 09:21
@ask-z4ch
fix: resolve pre-existing test-infrastructure bugs (Closes utksh1#365)
8c76e29 
- Add trio>=0.27.0 to requirements-dev.txt so anyio can parametrize
  tests over both backends without ModuleNotFoundError
- Pin anyio backends to ['asyncio'] via conftest fixture since the
  codebase uses asyncio.create_task directly and is not trio-compatible
- Add ignore_cleanup_errors=True to TemporaryDirectory in conftest.py
  to prevent Windows PermissionError on SQLite DB cleanup

Signed-off-by: ask-z4ch <229467675+ask-z4ch@users.noreply.github.com>
Closes utksh1#365
@ask-z4ch
fix(sandbox): harden sandbox executor with proactive streaming, memor…
58cd9ed 
…y classification, legacy compat (Closes utksh1#326)

Rewrite sandbox_executor.py per security specification:
- Proactive 64KB chunked stream reader for stdout+stderr (no communicate())
- Output limit detection stops reading immediately, terminates process
- Timeout enforcement via asyncio.wait_for with SIGTERM->3s->SIGKILL escalation
- Memory exhaustion heuristics: SIGSEGV(-11/139), MemoryError string, RSS>=95%
- Platform guard: RLIMIT_AS applied only on Linux, graceful no-op elsewhere
- try/finally cancellation guard ensures subprocess is never orphaned

Core Executor Adapter:
- _execute_command signature preserved: (cmd, timeout=None) -> (output, exit_code, violation_reason)
- SandboxConfig created internally from global settings
- Per-plugin sandbox config handled at execute_task level via timeout override
- Returns violation_reason string directly for terminated:* status mapping

Models:
- SandboxViolation changed to Exception subclass (was BaseModel)
- SandboxConfig defaults unchanged

Tests:
- 16 tests covering: legacy timeout compatibility, signal escalation,
  memory classification (SIGSEGV, MemoryError, RSS), output truncation,
  cancellation safety, platform guard, normal completion, config resolution

Signed-off-by: ask-z4ch <229467675+ask-z4ch@users.noreply.github.com>
Closes utksh1#326
@ask-z4ch
fix(executor): restore legacy _execute_command(command, task_id, time…
1a0ca37 
…out=600) signature

Preserve original 3-parameter signature for backward compatibility.
task_id is accepted but only used for parameter compatibility;
SandboxConfig is created internally from the timeout parameter.

Signed-off-by: ask-z4ch <229467675+ask-z4ch@users.noreply.github.com>
@ask-z4ch
Merge branch 'main' into main
4d11f36
@ask-z4ch ask-z4ch marked this pull request as draft May 29, 2026 05:53
@ask-z4ch ask-z4ch marked this pull request as ready for review May 29, 2026 05:53
@ask-z4ch ask-z4ch requested a review from utksh1 May 29, 2026 05:56
@ask-z4ch
Copy link
Copy Markdown
Contributor Author

ask-z4ch commented May 29, 2026

Closing and reopening to force GitHub to re-evaluate mergeability after rebase. No code changes.

@ask-z4ch ask-z4ch closed this May 29, 2026
@ask-z4ch ask-z4ch reopened this May 29, 2026
@ask-z4ch ask-z4ch closed this May 29, 2026
@ask-z4ch
Copy link
Copy Markdown
Contributor Author

ask-z4ch commented May 29, 2026 •
edited
Loading

@utksh1
I don't understand how come backtests are failing in this PR.
I've opened a new #408

@utksh1 utksh1 added the gssoc:invalid Admin validation: invalid for GSSoC scoring label May 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@utksh1 utksh1 Awaiting requested review from utksh1

Assignees

No one assigned

Labels

area:backend Backend API, database, or service work area:plugins Scanner plugin metadata, schemas, or plugin runtime work area:security Security-sensitive implementation or tests gssoc:invalid Admin validation: invalid for GSSoC scoring level:critical 80 pts difficulty label for critical or high-impact PRs type:feature Feature work category bonus label type:security Security work category bonus label type:testing Testing work category bonus label

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[FEAT] Add plugin execution sandbox with resource limits and timeout enforcement

2 participants

@ask-z4ch @utksh1