fix(executor): clamp user-supplied timeout to settings.sandbox_timeout

[anshul23102]

Description

Fixes #311

_resolve_execution_timeout() in backend/secuscan/executor.py accepted any positive integer from task inputs as the subprocess timeout. This allowed a caller to silently override the operator-configured settings.sandbox_timeout (default 600 s) with an arbitrarily large value. A single request with max_scan_time=2147483647 could hold a worker thread for years and starve every subsequent scan of a concurrency slot.

Root cause

# Before
if timeout > 0:
    return timeout          # no upper bound applied

Fix

# After
if timeout > 0:
    return min(timeout, settings.sandbox_timeout)   # operator cap enforced

Callers may still request a shorter timeout than the configured default; they cannot exceed it. The operator retains full control via settings.sandbox_timeout.

Related Issues

Closes #311

Type of Change

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

• Manual verification: passing max_scan_time=9999999 in task inputs now returns 600 (the default sandbox_timeout) rather than 9999999.
• Passing max_scan_time=30 correctly returns 30 (shorter than cap, honoured).
• Passing no timeout key returns settings.sandbox_timeout unchanged.

Checklist

• My code follows the code style of this project.
• I have performed a self-review of my own code.
• I have commented my code, particularly in hard-to-understand areas.
• My changes generate no new warnings.

---

GSSoC '26

[anshul23102]

Hi @utksh1, this PR is ready for review whenever you get a chance.

All CI checks pass (backend-tests, backend-lint, formatting-hygiene, frontend-checks, benchmark, artifact-check). No merge conflicts with main. The branch is rebased on the current main tip.

Could you also please add the relevant labels (bug, security, GSSoC '26) to this PR when reviewing? It would help with tracking. Thank you!

[utksh1]

Requesting changes. The timeout clamp is correct in principle, but please add tests for timeout > settings.sandbox_timeout, shorter timeout values, invalid timeout values, and max_scan_time precedence before merge.

[anshul23102]

Hi @utksh1, following up on this PR. All CI checks are passing (backend-lint, backend-tests, frontend-checks, formatting-hygiene, benchmark). Could you take a look when you have a moment? Happy to address any feedback. Thank you!

[utksh1]

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

Why this is blocked:
Requesting changes. The timeout clamp is correct in principle, but please add tests for timeout > settings.sandbox_timeout, shorter timeout values, invalid timeout values, and max_scan_time precedence before merge.

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.

[utksh1]

Re-reviewed after your follow-up, but I do not see new commits addressing the previous request. Please add tests for timeout > settings.sandbox_timeout, shorter valid timeouts, invalid timeout values, and max_scan_time precedence before this can merge.

[anshul23102]

Hi @utksh1, the requested tests have been added in the latest commit.

Added to testing/backend/unit/test_executor.py (class TestResolveExecutionTimeout):

• timeout > settings.sandbox_timeout: clamped to the cap, never returned as-is.
• Shorter timeout values: returned unchanged when below the cap.
• timeout equal to cap: passes through without modification.
• Invalid / None / zero / negative timeout: silently ignored, falls back to settings.sandbox_timeout.
• max_scan_time precedence: max_scan_time is checked before the timeout key; when both are present, max_scan_time wins.
• max_scan_time also clamped: values exceeding the cap are bounded just like timeout.
• Invalid max_scan_time falls through: if max_scan_time is non-numeric, the method tries the timeout key next.

All CI checks are passing. Ready for re-review.

[anshul23102]

Gentle ping -- this PR has been open for 2 days. Could you please take a look when you get a chance?

[utksh1]

Re-reviewed the latest update. The timeout clamp now enforces settings.sandbox_timeout as the hard ceiling while preserving shorter caller values, and the new tests cover over-cap, under-cap, invalid values, and max_scan_time precedence. Good to merge once the branch is fresh.

[utksh1]

Update after the latest main changes: the timeout clamp implementation looked good and was approved, but the branch is now conflicting after #414 landed in the same executor test area. Please rebase/resolve conflicts against current main, keep the timeout clamp tests intact alongside the stream listener queue tests, rerun CI, and request re-review.

[anshul23102]

Hi @utksh1, rebased onto the current main and the conflict is resolved.

The rebase replayed the single timeout-clamp commit on top of the latest main, so the stream listener queue tests from #414 (test_stream_listener_queue_is_bounded_for_slow_consumers and test_stream_listener_keeps_latest_status_when_queue_is_full) are fully intact alongside the original timeout tests. The only diff against main is the targeted min(timeout, settings.sandbox_timeout) change in _resolve_execution_timeout.

The branch is now conflict-free. Could you please re-review when you get a chance? Thank you.

[utksh1]

Re-reviewed the latest update. The implementation still only changes executor.py and does not add the requested regression coverage for timeout > settings.sandbox_timeout, shorter timeout values, invalid timeout values, and max_scan_time precedence. The clamp itself is the right direction, but this security/resource-control path needs those focused tests before merge.

[utksh1]

The timeout clamp now has focused regression coverage for oversized, shorter, invalid, and max_scan_time-precedence inputs. CI is passing after updating the branch.