Codejail safe_exec makes "unsafe=true" decision at startup #225
Comments
timmc-edx
added a commit
to timmc-edx/codejail-service
that referenced
this issue
Feb 6, 2025
- Initialize codejail at startup, if `CODE_JAIL` is set - Run safety checks at startup, locking out the API if the checks fail If codejail isn't properly configured, it defaults to running code unsafely. To prevent this from affecting the service, we run a smoke test at startup to check if there's anything just *drastically* wrong. If this check does not pass, two things happen: - The healthcheck endpoint will never return a 200 OK - The code-exec endpoint will refuse with a 500 error Supporting changes: - Define an explicit AppConfig for the api subpackage so that we can hook into the `ready()` mechanism - Wrap `safe_exec` to prevent codejail eagerly setting `UNSAFE=True` at module load time. (Not clear why this doesn't affect edx-platform; maybe something to do with app vs. middleware load order.) Filed openedx/codejail#225 for possibly fixing this. - `safe_exec` wrapper also provides deepcopy to allow callers to reason about the globals dict more easily. Other changes: - Clean up healthcheck docstring (mostly just trim it down) - Lint cleanup Part of edx/edx-arch-experiments#927
timmc-edx
added a commit
to timmc-edx/codejail-service
that referenced
this issue
Feb 6, 2025
- Initialize codejail at startup, if `CODE_JAIL` is set - Run safety checks at startup, locking out the API if the checks fail If codejail isn't properly configured, it defaults to running code unsafely. To prevent this from affecting the service, we run a smoke test at startup to check if there's anything just *drastically* wrong. If this check does not pass, two things happen: - The healthcheck endpoint will never return a 200 OK - The code-exec endpoint will refuse with a 500 error Supporting changes: - Define an explicit AppConfig for the api subpackage so that we can hook into the `ready()` mechanism - Wrap `safe_exec` to prevent codejail eagerly setting `UNSAFE=True` at module load time. (Not clear why this doesn't affect edx-platform; maybe something to do with app vs. middleware load order.) Filed openedx/codejail#225 for possibly fixing this. - `safe_exec` wrapper also performs a deepcopy to allow callers to reason about the globals dict more easily. Other changes: - Clean up healthcheck docstring (mostly just trim it down) - Lint cleanup Part of edx/edx-arch-experiments#927
timmc-edx
added a commit
to timmc-edx/codejail-service
that referenced
this issue
Feb 6, 2025
- Initialize codejail at startup, if `CODE_JAIL` is set - Run safety checks at startup, locking out the API if the checks fail If codejail isn't properly configured, it defaults to running code unsafely. To prevent this from affecting the service, we run a smoke test at startup to check if there's anything just *drastically* wrong. If this check does not pass, two things happen: - The healthcheck endpoint will never return a 200 OK - The code-exec endpoint will refuse with a 500 error Supporting changes: - Define an explicit AppConfig for the api subpackage so that we can hook into the `ready()` mechanism - Wrap `safe_exec` to prevent codejail eagerly setting `UNSAFE=True` at module load time. (Not clear why this doesn't affect edx-platform; maybe something to do with app vs. middleware load order.) Filed openedx/codejail#225 for possibly fixing this. - `safe_exec` wrapper also performs a deepcopy to allow callers to reason about the globals dict more easily. Other changes: - Clean up healthcheck docstring (mostly just trim it down) - Lint cleanup Part of edx/edx-arch-experiments#927
6 tasks
timmc-edx
added a commit
to timmc-edx/codejail-service
that referenced
this issue
Feb 6, 2025
- Initialize codejail at startup, if `CODE_JAIL` is set - Run safety checks at startup, locking out the API if the checks fail If codejail isn't properly configured, it defaults to running code unsafely. To prevent this from affecting the service, we run a smoke test at startup to check if there's anything just *drastically* wrong. If this check does not pass, two things happen: - The healthcheck endpoint will never return a 200 OK - The code-exec endpoint will refuse with a 500 error Supporting changes: - Define an explicit AppConfig for the api subpackage so that we can hook into the `ready()` mechanism - Wrap `safe_exec` to prevent codejail eagerly setting `UNSAFE=True` at module load time. (Not clear why this doesn't affect edx-platform; maybe something to do with app vs. middleware load order.) Filed openedx/codejail#225 for possibly fixing this. - `safe_exec` wrapper also performs a deepcopy to allow callers to reason about the globals dict more easily. Other changes: - Clean up healthcheck docstring (mostly just trim it down) - Lint cleanup Part of edx/edx-arch-experiments#927
timmc-edx
added a commit
to timmc-edx/codejail-service
that referenced
this issue
Feb 6, 2025
- Initialize codejail at startup, if `CODE_JAIL` is set - Run safety checks at startup, locking out the API if the checks fail If codejail isn't properly configured, it defaults to running code unsafely. To prevent this from affecting the service, we run a smoke test at startup to check if there's anything just *drastically* wrong. If this check does not pass, two things happen: - The healthcheck endpoint will never return a 200 OK - The code-exec endpoint will refuse with a 500 error Supporting changes: - Define an explicit AppConfig for the api subpackage so that we can hook into the `ready()` mechanism - Wrap `safe_exec` to prevent codejail eagerly setting `UNSAFE=True` at module load time. (Not clear why this doesn't affect edx-platform; maybe something to do with app vs. middleware load order.) Filed openedx/codejail#225 for possibly fixing this. - `safe_exec` wrapper also performs a deepcopy to allow callers to reason about the globals dict more easily. Other changes: - Clean up healthcheck docstring (mostly just trim it down) - Lint cleanup Part of edx/edx-arch-experiments#927
timmc-edx
added a commit
to timmc-edx/codejail-service
that referenced
this issue
Feb 6, 2025
- Initialize codejail at startup, if `CODE_JAIL` is set - Run safety checks at startup, locking out the API if the checks fail If codejail isn't properly configured, it defaults to running code unsafely. To prevent this from affecting the service, we run a smoke test at startup to check if there's anything just *drastically* wrong. If this check does not pass, two things happen: - The healthcheck endpoint will never return a 200 OK - The code-exec endpoint will refuse with a 500 error Supporting changes: - Define an explicit AppConfig for the api subpackage so that we can hook into the `ready()` mechanism - Wrap `safe_exec` to prevent codejail eagerly setting `UNSAFE=True` at module load time. (Not clear why this doesn't affect edx-platform; maybe something to do with app vs. middleware load order.) Filed openedx/codejail#225 for possibly fixing this. - `safe_exec` wrapper also performs a deepcopy to allow callers to reason about the globals dict more easily. Other changes: - Clean up healthcheck docstring (mostly just trim it down) - Lint cleanup Part of edx/edx-arch-experiments#927
timmc-edx
added a commit
to timmc-edx/codejail-service
that referenced
this issue
Feb 11, 2025
- Initialize codejail at startup - Run safety checks at startup, locking out the API if the checks fail If codejail isn't properly configured, it defaults to running code unsafely. To prevent this from affecting the service, we run a smoke test at startup to check if there's anything just *drastically* wrong. If this check does not pass, two things happen: - The healthcheck endpoint will never return a 200 OK - The code-exec endpoint will refuse with a 500 error Supporting changes: - Define an explicit AppConfig for the api subpackage so that we can hook into the `ready()` mechanism - Wrap `safe_exec` to prevent codejail eagerly setting `UNSAFE=True` at module load time. (Not clear why this doesn't affect edx-platform; maybe something to do with app vs. middleware load order.) Filed openedx/codejail#225 for possibly fixing this. - `safe_exec` wrapper also performs a deepcopy to allow callers to reason about the globals dict more easily. Other changes: - Clean up healthcheck docstring (mostly just trim it down) - Lint cleanup Part of edx/edx-arch-experiments#927
|
Turns out this is a duplicate -- see older issue #16 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When the
codejail.safe_execmodule is imported, it decides immediately whether to run in unsafe mode:codejail/codejail/safe_exec.py
Line 293 in 0165d9c
This means that codejail must be fully configured before it is imported. Any module that imports this one (usually for
safe_execorSafeExecException) must itself be imported after this point. This might work if the relying code is a Django app and if theConfigureCodeJailMiddlewareis in effect, but a small change could still cause codejail to start loading in unsafe mode.It would likely be better to have the
safe_execfunction decide at call-time whether to branch tonot_safe_exec, as this would allow service startup to occur in any order (as long ascodejail.jail_code.configureis called at some point beforesafe_execstarts being called).This change could be made as part of an unsafe-mode deprecation, which would likely entail requiring an explicit opt-in for unsafe-exec (only used for unit tests) rather than defaulting to it.
The text was updated successfully, but these errors were encountered: