Skip to content

fix(examples): strengthen example agent code security by adding input validation and safe file extraction, and security notes to agent app docstring and example agent Readme - #95

Merged
lyzustc merged 1 commit into
awslabs:mainfrom
lyzustc:main
Jul 31, 2026

Conversation

@lyzustc

@lyzustc lyzustc commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR hardens the example agents so that untrusted input, such as request payloads and
data fetched from S3 (task configs, repo/testbed tarballs), is validated before it reaches an agent,
the filesystem, or an expression evaluator. All changes are behavior-preserving on valid inputs.

Math agent (examples/strands_math_agent)

  • Added models.py with a pydantic InvocationRequest model whose prompt field is
    typed str (plus optional answer).
  • basic_app.py and rl_app.py now construct InvocationRequest(**payload) and read
    request.prompt instead of payload.get("prompt"), so non-string payload values are
    rejected before the agent runs.
  • README: added a note under the Invoke section explaining that the entrypoint validates
    the payload and that the prompt: str field should be preserved when adapting the
    example.

OfficeBench agent (examples/strands_officebench_agent)

  • Safe comparator evaluation (reward.py). Replaced eval(match["comparator"])(value)
    in evaluate_excel_cell_comparator with _make_safe_comparator(), a small AST-based
    interpreter. It accepts only a single-argument lambda built from comparisons
    (including in/not in), boolean/arithmetic/unary ops, numeric/string literals,
    list/tuple/set literals, the lambda parameter, and a fixed allowlist of pure numeric
    builtins (int, float, str, len, abs, round, bool). Any other construct
    (attribute access, arbitrary names/calls, **, etc.) raises UnsafeComparatorError,
    which is logged and scored 0.0.
  • Task config validation (models.py, utils.py). Added pydantic TaskConfig and
    EvaluationCheck models. load_task_from_s3 now validates the downloaded JSON against
    TaskConfig (requiring task to be a string and a well-formed evaluation list) before
    returning it.
  • Safe tarball extraction (utils.py). setup_testbed now extracts the testbed
    archive with tarfile.extractall(..., filter="data"), rejecting members with absolute
    paths or .. traversal that would escape /testbed.
  • README: added a note at the end of the "Run benchmark" section explaining where
    task_uri/testbed_uri come from, how they flow into the invocation payload, and the
    trust-boundary expectation that they shoud point to user-controlled S3 buckets.

Migration agent (examples/strands_migration_agent)

  • Safe tarball extraction (utils.py). load_repo_from_s3 now extracts the repo
    archive with tarfile.extractall(..., filter="data"), preventing a crafted tar from
    escaping the work directory.
  • models.py. Documented that InvocationRequest.prompt is intentionally typed str
    and should not be relaxed, since it is passed to an agent with shell + editor tools.
  • README: added a note explaining that repo_uri is a trust boundary and should point to
    user-controlled S3 buckets.

Toolkit (src/agentcore_rl_toolkit/app.py)

  • Expanded the rollout_entrypoint docstring to state that payload validation is the
    handler's responsibility (the decorator is framework-agnostic plumbing and performs no
    sanitization) and to recommend the pydantic prompt: str pattern, pointing to
    examples/strands_math_agent/models.py as the reference.

… validation and safe file extraction, and security notes to agent app docstring and example agent Readme
"""Raised when a comparator string contains constructs outside the allowlist."""


def _make_safe_comparator(expr: str):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a fairly high maintenance cost to ensure that this function is correct and does not miss some edge cases. Instead, this could be a good fit for a sandboxed function execution (AWS Lambda, AgentCore Code Interpreter, Firecracker, Docker, etc).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion! Actually the goal is not to cover every edge case, but primarily block one specific escape. ACR is already a Firecracker sandbox, so arbitrary code in the reward is contained by default. What impacts is one way to reach outside of the sandbox by AWS role attack: untrusted code reads the session's execution-role credentials (via MMDSv2, like EC2 instance metadata) and acts against other AWS services / exfiltrates. We don't need to be perfect on edge cases; we need to make sure malformed code can't grab the role and leave.

Doing sandboxed reward code execution with AWS Lambda or AgentCore Code Interpreter is an alternative. However, this reward function doesn't just run a comparator — it reads the whole post-rollout testbed off disk (Excel, Word, PDF, email, calendar) across 9 functions. A separate sandbox has its own filesystem, so for every rollout we'd marshal that entire binary tree in and port two eval paths whose libs aren't preinstalled. This cause a large time cost to RL training.

Hence, directly inspecting the code itself is the easiest fix to solve the primary vulnerability. Using the implemented AST allowlist (single-arg lambda over comparisons/membership, arithmetic, literals; anything else rejected) sets a strict security gate preventing AWS role attack from happening with little time cost. Other edge cases not related to escaping Bedrock AgentCore do not make real harms.

@lyzustc
lyzustc merged commit 712fe7d into awslabs:main Jul 31, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants