fix: conservative trust-boundary hardening (#540) - #551
Open
Shashankss1205 wants to merge 1 commit into
Open
Conversation
Non-controversial parts of the trust-boundary issue (instantiate sandboxing,
BUG-09, is intentionally left for a maintainer decision — the server already
exposes run_command):
- BUG-11: call_method now blocks private/dunder methods (allowlist of __call__,
__len__, __repr__, __str__). __reduce__ previously dumped __dict__ including
the fitted _y/_X training data to any caller.
- BUG-10: instantiate rejects specs that don't produce an sktime object
("42" -> int, "[1,2,3]" -> list); they used to get est_ handles that failed
confusingly downstream.
- BUG-12: corrected run_command's description — it runs on the HOST as the
server user, not "inside the sktime container".
- BUG-23: run_command output is capped (~20k chars) with a truncated flag, and
a non-zero exit now includes an "error" key like every other tool.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Addresses #540 — the conservative, non-controversial parts of the trust-boundary umbrella.
Scope note
BUG-09 (sandboxing
instantiate/craft) is intentionally left out. The server already exposesrun_command(arbitrary host execution by design), so the caller is nominally trusted; locking downcraftspecs with an AST allowlist risks breaking legitimate composites and is a posture decision for the maintainers. This PR does the parts that are pure improvements regardless of posture.BUG-11 — call_method exposed private/dunder methods
call_method(method_name="__reduce__")returned the full__dict__, dumping the fitted_y/_Xtraining data to any caller.call_methodnow rejects anymethod_namestarting with_, except a small allowlist (__call__for callable metrics/aligners,__len__,__repr__,__str__). Public methods are unaffected.BUG-10 — instantiate accepted non-estimators
instantiate("42")→ anest_handle of typeintthat failed confusingly downstream. Aftercraft, the result must be an sktime object (skbaseBaseObject, or duck-typedget_params+get_class_tag); otherwise a clear "Spec did not produce an sktime estimator, got int".BUG-12 — run_command description was false
It claimed the command runs "inside the sktime container". Corrected to say it runs on the host, in the server's working directory, as the server user.
BUG-23 — run_command output was unbounded
seq 1 100000returned ~689k chars. Output is capped at ~20k chars (head+tail) with atruncatedflag, and a non-zero exit now includes an"error"key like every other tool (also closes N-24).Testing
tests/test_security_hardening.py(14 tests):__reduce__/__class__/private methods blocked, public methods work;42/[1,2,3]/Nonerejected, real estimator still instantiates; large output truncated, small output intact, non-zero exit haserror.🤖 Generated with Claude Code