Add audit hook-based sandbox for Python workers by benoitc · Pull Request #2 · benoitc/erlang-python

benoitc · 2026-02-20T01:02:02Z

Summary

Implement sandboxing for Python workers using Python's audit hook mechanism (PEP 578)
Add per-worker configuration of blocked operations (file_write, subprocess, network, ctypes, import, exec)
Add strict preset that blocks subprocess, network, ctypes, and file_write operations
Add high-level py:call_sandboxed/4,5 API for easy sandboxed execution
Add dynamic enable/disable support and import whitelist feature
Add documentation in docs/sandbox.md

API

%% Create worker with strict preset (blocks subprocess, network, ctypes, file_write)
{ok, Worker} = py_nif:worker_new(#{
    sandbox => #{preset => strict}
}).

%% Or explicit policy
{ok, Worker} = py_nif:worker_new(#{
    sandbox => #{
        enabled => true,
        block => [file_write, subprocess, network],
        allow_imports => [<<"json">>, <<"math">>],
        log_events => true
    }
}).

%% Dynamic policy control
py_nif:sandbox_set_policy(WorkerRef, Policy).
py_nif:sandbox_enable(WorkerRef, true | false).

%% High-level API
{ok, Result} = py:call_sandboxed(Module, Func, Args, SandboxOpts).

Notes

File write blocking (open with w/a/x/+ modes) is fully functional
Subprocess and network blocking depends on Python version audit event support
Tests use file operations for verification as they are reliably audited across Python versions

Implement sandboxing for Python workers using Python's audit hook mechanism (PEP 578). This allows per-worker configuration of blocked operations: - file_write: Blocks open() with write/append/create modes - file_read: Blocks all file read operations - subprocess: Blocks subprocess.Popen, os.system, os.exec*, os.spawn* - network: Blocks socket.* operations - ctypes: Blocks ctypes module (memory access) - import: Blocks non-whitelisted imports - exec: Blocks compile(), exec(), eval() Features: - Strict preset (blocks subprocess, network, ctypes, file_write) - Per-worker sandbox policy with dynamic enable/disable - Import whitelist support - Thread-safe policy updates via atomic flags and mutex - High-level py:call_sandboxed/4,5 API - Documentation in docs/sandbox.md

Copilot

Pull request overview

This pull request implements a comprehensive sandboxing mechanism for Python workers using Python's PEP 578 audit hook system. The implementation adds per-worker configuration for blocking potentially dangerous operations such as file writes, subprocess execution, network access, ctypes usage, dynamic code execution, and imports. A "strict" preset is provided that blocks subprocess, network, ctypes, and file write operations. The PR includes both low-level NIF functions and a high-level py:call_sandboxed/4,5 API for convenient sandboxed execution.

Changes:

Added audit hook-based sandboxing infrastructure in C (py_sandbox.c/h) with configurable block flags and import whitelist support
Extended worker management to support sandbox policy initialization, updates, and queries via new NIF functions
Provided high-level Erlang API for one-off sandboxed function calls with automatic worker cleanup
Added comprehensive test suite and documentation

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
c_src/py_sandbox.h	Header file defining sandbox policy structure, block flags, presets, and API functions
c_src/py_sandbox.c	Implementation of audit hook mechanism, policy management, and event classification
c_src/py_nif.h	Added forward declaration for sandbox_policy_t and sandbox field to py_worker_t struct
c_src/py_nif.c	Integrated sandbox initialization, worker creation/destruction with policy, and NIF functions
src/py_nif.erl	Exported new sandbox control NIFs (set_policy, enable, get_policy) with stubs
src/py.erl	Added high-level call_sandboxed/4,5 API for convenient sandboxed execution
test/py_sandbox_SUITE.erl	Comprehensive test suite covering presets, explicit policies, dynamic control, and escape attempts
docs/sandbox.md	Complete documentation with examples, API reference, limitations, and architecture overview

Comments suppressed due to low confidence (1)

c_src/py_nif.c:614

Missing error handling for Python object creation: If PyThreadState_New (line 589), PyDict_New (lines 592-593), or subsequent Python operations fail after the sandbox policy is created, the function continues without checking for errors and without cleaning up the allocated sandbox policy. This could lead to a memory leak. Add NULL checks after each Python API call that can fail, and if any fail, destroy the sandbox policy, release the GIL, release the worker resource, and return an appropriate error.

    /* Acquire GIL to create thread state */
    PyGILState_STATE gstate = PyGILState_Ensure();

    /* Create a new thread state for this worker */
    PyInterpreterState *interp = PyInterpreterState_Get();
    worker->thread_state = PyThreadState_New(interp);

    /* Create global/local namespaces */
    worker->globals = PyDict_New();
    worker->locals = PyDict_New();

    /* Import __builtins__ into globals */
    PyObject *builtins = PyEval_GetBuiltins();
    PyDict_SetItemString(worker->globals, "__builtins__", builtins);

    /* Import erlang module into worker's namespace for callbacks */
    PyObject *erlang_module = PyImport_ImportModule("erlang");
    if (erlang_module != NULL) {
        PyDict_SetItemString(worker->globals, "erlang", erlang_module);
        Py_DECREF(erlang_module);
    }

    worker->owns_gil = false;

    /* Initialize callback state */
    worker->callback_pipe[0] = -1;
    worker->callback_pipe[1] = -1;
    worker->has_callback_handler = false;
    worker->callback_env = NULL;

    PyGILState_Release(gstate);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-20T01:16:44Z

c_src/py_sandbox.c

+        /* Check if it's a submodule of an allowed package */
+        if (strncmp(module_name, allowed, allowed_len) == 0 &&
+            module_name[allowed_len] == '.') {
+            return true;


Potential bypass in import whitelist: The submodule check on lines 211-213 only checks if the module name starts with an allowed prefix followed by a dot. This could allow bypassing the whitelist if a malicious module name starts with an allowed name. For example, if "os" is whitelisted, "os_evil" would not match, but "os.evil" would be allowed. While this is likely the intended behavior for legitimate submodules, consider documenting this behavior clearly to avoid confusion about what gets whitelisted.

Copilot · 2026-02-20T01:16:44Z

c_src/py_sandbox.c

+    memset(policy, 0, sizeof(sandbox_policy_t));
+    pthread_mutex_init(&policy->mutex, NULL);
+    policy->enabled = false;
+    policy->block_flags = 0;


Incorrect initialization of atomic variable: On line 402, policy->block_flags is initialized using a simple assignment = 0, but block_flags is declared as _Atomic uint32_t (see py_sandbox.h line 109). Atomic variables should be initialized using atomic_init(&policy->block_flags, 0) or ATOMIC_VAR_INIT(0) to ensure proper atomic initialization according to C11 standards. While simple assignment often works in practice, using the proper atomic initialization function is more portable and correct.

Copilot · 2026-02-20T01:16:44Z

docs/sandbox.md

+|------|--------|
+| `file_write` | `open()` with write/append modes |
+| `file_read` | All file read operations |
+| `subprocess` | `subprocess.*`, `os.exec*`, `os.spawn*`, `os.popen` |


Documentation incomplete for subprocess blocking: The documentation states that the subprocess flag blocks "subprocess., os.exec, os.spawn*, os.popen" (line 70), but the implementation also blocks os.fork and os.posix_spawn (see c_src/py_sandbox.c lines 122-123). Update the documentation to include these additional blocked operations for completeness.

Copilot · 2026-02-20T01:16:45Z

src/py.erl

+%% '''
+%%
+%% With strict preset, the following are blocked:
+%% - subprocess: subprocess.*, os.system, os.exec*, os.spawn*, os.popen


Documentation incomplete for subprocess blocking in py.erl: The documentation comment on line 844 lists subprocess operations blocked by the strict preset but omits os.fork and os.posix_spawn which are also blocked according to c_src/py_sandbox.c lines 122-123. Update the documentation to include these for completeness and consistency with the implementation.

Suggested change

%% - subprocess: subprocess.*, os.system, os.exec*, os.spawn*, os.popen

%% - subprocess: subprocess.*, os.system, os.exec*, os.spawn*, os.popen, os.fork, os.posix_spawn

Copilot · 2026-02-20T01:16:45Z

test/py_sandbox_SUITE.erl

+%%% ============================================================================
+
+%% Test various file write escape attempts
+test_subprocess_escape_attempts(_Config) ->


The test function test_subprocess_escape_attempts is misleadingly named. The test actually validates file write blocking with different file modes (w, a, x, w+), not subprocess escape attempts. Consider renaming to test_file_write_escape_attempts to accurately reflect what is being tested.

Suggested change

test_subprocess_escape_attempts(_Config) ->

test_file_write_escape_attempts(_Config) ->

Copilot · 2026-02-20T01:16:46Z

test/py_sandbox_SUITE.erl

+    ok.
+
+%% Test that file reads are allowed
+test_network_escape_attempts(_Config) ->


The test function test_network_escape_attempts is misleadingly named. The test actually validates that file read operations are allowed (testing the inverse of file write blocking), not network escape attempts. Consider renaming to test_file_read_allowed to accurately reflect what is being tested.

Suggested change

test_network_escape_attempts(_Config) ->

test_file_read_allowed(_Config) ->

Copilot · 2026-02-20T01:16:46Z

c_src/py_sandbox.c

+                    if (import_name != NULL) {
+                        memcpy(import_name, bin.data, bin.size);
+                        import_name[bin.size] = '\0';
+                        policy->allowed_imports[policy->allowed_imports_count++] = import_name;
+                    }
+                } else {
+                    /* Try as atom */
+                    char atom_buf[256];
+                    if (enif_get_atom(env, head, atom_buf, sizeof(atom_buf), ERL_NIF_LATIN1)) {
+                        char *import_name = enif_alloc(strlen(atom_buf) + 1);
+                        if (import_name != NULL) {
+                            strcpy(import_name, atom_buf);
+                            policy->allowed_imports[policy->allowed_imports_count++] = import_name;
+                        }


Memory leak when allocating import whitelist: If allocation fails for an individual import_name on lines 527 or 537, the function continues without adding it to the array, but previously allocated import names in policy->allowed_imports are not freed if a subsequent failure occurs, and the partially populated array is not cleaned up on error. Consider adding error handling to free all previously allocated import names and the array itself if any allocation fails.

Suggested change

if (import_name != NULL) {

memcpy(import_name, bin.data, bin.size);

import_name[bin.size] = '\0';

policy->allowed_imports[policy->allowed_imports_count++] = import_name;

}

} else {

/* Try as atom */

char atom_buf[256];

if (enif_get_atom(env, head, atom_buf, sizeof(atom_buf), ERL_NIF_LATIN1)) {

char *import_name = enif_alloc(strlen(atom_buf) + 1);

if (import_name != NULL) {

strcpy(import_name, atom_buf);

policy->allowed_imports[policy->allowed_imports_count++] = import_name;

}

if (import_name == NULL) {

/* Cleanup previously allocated imports on failure */

unsigned int i;

for (i = 0; i < policy->allowed_imports_count; i++) {

enif_free(policy->allowed_imports[i]);

}

enif_free(policy->allowed_imports);

policy->allowed_imports = NULL;

policy->allowed_imports_count = 0;

return -1;

}

memcpy(import_name, bin.data, bin.size);

import_name[bin.size] = '\0';

policy->allowed_imports[policy->allowed_imports_count++] = import_name;

} else {

/* Try as atom */

char atom_buf[256];

if (enif_get_atom(env, head, atom_buf, sizeof(atom_buf), ERL_NIF_LATIN1)) {

char *import_name = enif_alloc(strlen(atom_buf) + 1);

if (import_name == NULL) {

/* Cleanup previously allocated imports on failure */

unsigned int i;

for (i = 0; i < policy->allowed_imports_count; i++) {

enif_free(policy->allowed_imports[i]);

}

enif_free(policy->allowed_imports);

policy->allowed_imports = NULL;

policy->allowed_imports_count = 0;

return -1;

}

strcpy(import_name, atom_buf);

policy->allowed_imports[policy->allowed_imports_count++] = import_name;

Copilot · 2026-02-20T01:16:47Z

c_src/py_sandbox.c

+    /* Update import whitelist */
+    if (temp.allowed_imports != NULL) {
+        /* Free old whitelist */
+        if (policy->allowed_imports != NULL) {
+            for (size_t i = 0; i < policy->allowed_imports_count; i++) {
+                enif_free(policy->allowed_imports[i]);
+            }
+            enif_free(policy->allowed_imports);
+        }
+        policy->allowed_imports = temp.allowed_imports;
+        policy->allowed_imports_count = temp.allowed_imports_count;
+    }


Incomplete policy update when allow_imports is not provided: In sandbox_policy_update, if the new options don't include an allow_imports list, the existing whitelist is not cleared. This means if a policy previously had an import whitelist and you update it without specifying allow_imports, the old whitelist remains active. This could be unexpected behavior. Consider either documenting this behavior or adding logic to clear the whitelist when updating to a policy that doesn't specify allow_imports.

Suggested change

/* Update import whitelist */

if (temp.allowed_imports != NULL) {

/* Free old whitelist */

if (policy->allowed_imports != NULL) {

for (size_t i = 0; i < policy->allowed_imports_count; i++) {

enif_free(policy->allowed_imports[i]);

}

enif_free(policy->allowed_imports);

}

policy->allowed_imports = temp.allowed_imports;

policy->allowed_imports_count = temp.allowed_imports_count;

}

/* Update import whitelist

*

* Always synchronize from temp so that omitting `allow_imports`

* in an update clears any existing whitelist.

*/

/* Free old whitelist */

if (policy->allowed_imports != NULL) {

for (size_t i = 0; i < policy->allowed_imports_count; i++) {

enif_free(policy->allowed_imports[i]);

}

enif_free(policy->allowed_imports);

}

/* Take ownership of temp.allowed_imports (may be NULL) */

policy->allowed_imports = temp.allowed_imports;

policy->allowed_imports_count = temp.allowed_imports_count;

Copilot · 2026-02-20T01:16:47Z

c_src/py_sandbox.c

+        unsigned int list_len;
+        if (enif_get_list_length(env, value, &list_len)) {
+            ERL_NIF_TERM head, tail = value;
+            policy->block_flags = 0;


Unexpected behavior when both preset and block list are specified: If both preset and block options are provided in the sandbox configuration, the block list processing on line 486 sets policy->block_flags = 0, which overwrites the preset flags set on line 466. This means the block list takes precedence and the preset is ignored. This behavior should either be documented clearly or changed to merge the preset with the explicit block list (e.g., using |= instead of = on line 486).

Suggested change

policy->block_flags = 0;

/* Do not reset block_flags here so presets (if any) are preserved. */

Copilot · 2026-02-20T01:16:47Z

c_src/py_sandbox.c

+    if (policy != NULL) {
+        policy->enabled = enabled;


Race condition in sandbox policy enable/disable: The sandbox_policy_set_enabled function modifies policy->enabled without acquiring the mutex, while other functions like sandbox_policy_update and sandbox_policy_to_term acquire the mutex when accessing policy fields. This creates a potential race condition where the enabled state could be read or written inconsistently. The function should lock the mutex before modifying the enabled field to ensure thread safety.

Suggested change

if (policy != NULL) {

policy->enabled = enabled;

if (policy != NULL) {

pthread_mutex_lock(&policy->mutex);

policy->enabled = enabled;

pthread_mutex_unlock(&policy->mutex);

- Use atomic_init() for proper C11 atomic initialization - Add mutex lock in sandbox_policy_set_enabled for thread safety - Fix memory leaks in import whitelist allocation (cleanup on failure) - Fix memory leak in sandbox_policy_update when parse fails - Always sync allow_imports so omitting it clears existing whitelist - Preserve preset flags when explicit block list provided (use |= semantics) - Remove unused audit_handler field from sandbox_policy_t - Add os.fork and os.posix_spawn to subprocess blocking docs - Rename test functions to match their actual behavior - Add temp file cleanup in test_sandbox_set_policy

When disable_builtins is set to true, dangerous Python builtins (exec, eval, compile, __import__, open) are removed from the worker's namespace at creation time. This provides defense-in-depth beyond audit hook blocking: - exec() raises NameError instead of being caught by audit hook - Works even if audit hooks are bypassed somehow - Simpler and more reliable than audit-based exec blocking Usage: {ok, W} = py_nif:worker_new(#{ sandbox => #{preset => strict, disable_builtins => true} }).

- Fix nif_finalize to not cleanup Python-side resources when Py_Finalize is disabled. Since Python persists, ASGI/WSGI scope state, sandbox state, and cached types must remain valid across application restarts. - Fix disable_builtins to restore removed builtins when worker is destroyed. This prevents global Python state corruption that affected subsequent workers and test suites. - Remove __import__ from DANGEROUS_BUILTINS list as removing it breaks Python's import machinery (PyImport_ImportModule C API). - Add sandbox_restore_builtins() function to save and restore original builtin values. - Add py_sandbox_e2e_SUITE with 11 end-to-end tests for sandbox functionality including concurrent workers and escape attempts. All 127 tests now pass with no suite interaction issues.

benoitc requested a review from Copilot February 20, 2026 01:09

Copilot started reviewing on behalf of benoitc February 20, 2026 01:10 View session

benoitc force-pushed the feature/worker-sandbox branch from ee89b19 to 5e6e26f Compare February 20, 2026 01:10

Copilot AI reviewed Feb 20, 2026

View reviewed changes

benoitc added 3 commits February 20, 2026 02:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Comments

Add audit hook-based sandbox for Python workers#2

Add audit hook-based sandbox for Python workers#2
benoitc wants to merge 4 commits intomainfrom
feature/worker-sandbox

benoitc commented Feb 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	%% - subprocess: subprocess., os.system, os.exec, os.spawn*, os.popen
	%% - subprocess: subprocess., os.system, os.exec, os.spawn*, os.popen, os.fork, os.posix_spawn

	test_subprocess_escape_attempts(_Config) ->
	test_file_write_escape_attempts(_Config) ->

	test_network_escape_attempts(_Config) ->
	test_file_read_allowed(_Config) ->

	policy->block_flags = 0;
	/* Do not reset block_flags here so presets (if any) are preserved. */

Uh oh!

Comments

Conversation

benoitc commented Feb 20, 2026

Summary

API

Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant