Apply security-hardening patch set - #22
Closed
rdasilveiracabral wants to merge 6 commits into
Closed
Conversation
Co-authored-by: Ricardo Silveira Cabral <rdasilveiracabral@users.noreply.github.com>
Co-authored-by: Ricardo Silveira Cabral <rdasilveiracabral@users.noreply.github.com>
…rker env, enforce broker visibility Co-authored-by: Ricardo Silveira Cabral <rdasilveiracabral@users.noreply.github.com>
Co-authored-by: Ricardo Silveira Cabral <rdasilveiracabral@users.noreply.github.com>
…l_timeout, and populated DEFAULT_RESTRICTED_IMPORTS Co-authored-by: Ricardo Silveira Cabral <rdasilveiracabral@users.noreply.github.com>
These tests exercise in-process execution internals (session_locals write-back, FakeLLMClient call counting, in-cell nested/standalone generation, current_call, ATIF trajectory structure) that differ under the forked sandbox worker. Pin them to the in-process backend so they keep validating that path; sandbox-mode coverage for these flows is tracked separately. Co-authored-by: Ricardo Silveira Cabral <rdasilveiracabral@users.noreply.github.com>
Collaborator
Author
|
Closing as stale — superseded by the security hardening work that was merged separately. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Applies the uploaded security-hardening patch series (see its
README) and aligns the test suite with the resulting behavior change. Each numbered patch is one commit; the changes are disjoint by file.unifiedllm/registry.py,viewer/trace_routes.py) — validatesapi_key_envnames, adds anallowed_env_varsallow-list toresolve_api_key_from_config, and constrains browser-supplied playground models to endpoint/api_key_env pairs already declared server-side.nooa-cli .../start_dev.py,viewer/main.py) — default bind moves to127.0.0.1, optional bearer-token auth (NOOA_VIEWER_AUTH_TOKEN) gated byensure_viewer_bind_is_safe, and bounded OTLP/journal ingestion (max body sizes, bounded queue with 503 backpressure, journal item cap).config/strategy_config.py,runtime/actor.py,runtime/restrictions.py,runtime/sandbox/executor.py,runtime/sandbox/worker.py,strategies/codeact.py) — makessandboxthe CodeAct default with acell_timeoutof 30s, populatesDEFAULT_RESTRICTED_IMPORTSwith host-capability modules, scrubs the worker environment of parent secrets, syncs per-cell namespace/Outinto the worker, and enforces agentdoc visibility on every broker path.Application notes
execute_code(..., builtins={**builtins, **session.session_locals})→run_cell(..., builtins=...)→ the new_namespace_updatesworker sync). Fuzz mis-placed twoexecutor.py/actor.pyhunks, which I reconstructed by hand.ruff checkandruff format --checkpass repo-wide.Test alignment
uv run pytest -m "not integration and not stress"→ 6528 passed, 4 skipped, 0 failed. The sandbox-default flip initially broke 20 tests; those are now addressed in two commits:CodeActConfigdefaults (execution_backend="sandbox",cell_timeout=30.0), and the populatedDEFAULT_RESTRICTED_IMPORTSconstant.execution_backend="inprocess". These validate in-process internals —session_localswrite-back,FakeLLMClientcall counting, in-cell nested/standalone generation,current_call, and ATIF trajectory structure — which differ under the forked sandbox worker (aFakeLLMClientand parent-side locals don't cross the fork). Pinning preserves their intent; sandbox-mode coverage for these flows is a follow-up.No source files were changed for the test alignment — only tests.
Behavior notes for reviewers
CodeActConfig()now runs cells in a forked, env-scrubbed worker with a 30s hard per-cell timeout and broker-visibility enforcement.01_first_generation_method.pyand06_tracing.pyboth produced correct output.BenchAgent._solve_tasksetsCodeActConfig(max_iterations=300, ...)withoutexecution_backend, so it now runs in sandbox. It mostly uses brokeredself.shell/self.repo(still execute in the parent), but the worker env scrub, host-import restrictions, and 30s per-cell timeout are behavioral risks that were not validated against a live benchmark — consider pinning it toinprocessor giving it an explicit sandbox config.DEFAULT_RESTRICTED_IMPORTSis currently inert by default.RestrictionsConfig.restricted_importsstill defaults tofrozenset()and nothing wires the populated constant in (it only applies viaset_restricted_imports(...)), so the patch's "block host imports by default" effect is not actually active as written againstmain. If that effect is intended, the field default needs to referenceDEFAULT_RESTRICTED_IMPORTS(and expect further test fallout). Flagging rather than changing it unilaterally.Verification
uv run ruff check .→ all checks passed.uv run ruff format --check .→ clean (871 files).uv run pytest -m "not integration and not stress"→ 6528 passed, 4 skipped.