Skip to content

πŸ› fix(crux): bound agentstep prompt_path and answer file reads - #17362

Merged
hivecommons-hive[bot] merged 2 commits into
v5from
sec/fix-agentstep-file-bounds
Nov 9, 2026
Merged

hivecommons-hive[bot] merged 2 commits into
v5from
sec/fix-agentstep-file-bounds

Conversation

@hivecommons-hive

Copy link
Copy Markdown
Contributor

Security Fix

The step service (src/crux/agentstep) bounds every other caller- or agent-shaped input β€” inline prompt, schema, signals, request body β€” but read two workspace files whole into the crux server's memory: the run owner's prompt_path file and the codex answer file the sandboxed agent writes. From inside a contained run either could make the host allocate without limit, which is the boundary the package's os.OpenRoot work exists to hold.

Both reads now go through one readBounded helper that stops at a new exported MaxPromptFileBytes (16 MiB β€” 16Γ— the inline bound, so nobody using prompt_path for a large prompt today is affected; MaxPromptBytes is unchanged):

  • an oversize prompt_path is a *RequestError naming the bound, the same shape as the inline-prompt refusal;
  • an oversize answer file is reported to the backend as a failed read, which codexBackend.Parse already treats as "no file" and falls back to the last agent_message on stdout β€” no schema, status or wire change.

Tests use sparse files (os.Truncate) so they exercise the bound without 16 MiB of writes; both fail on the previous code. Changelog fragment under security.

Fixes #17361


Filed by sec-check agent (ACMM L6 β€” full mode)

β€” hive: agent=sec-check backend=copilot model=claude-fable-5.1 copilot=1.0.88

The step service bounds every other caller- or agent-shaped input
(inline prompt, schema, signals, request body) but read two workspace
files whole into host memory: the run owner's prompt_path file and the
codex answer file the sandboxed agent writes. Either could make the
crux server allocate without limit from inside a contained run.

Both reads now go through readBounded, which stops at
MaxPromptFileBytes (16 MiB): an oversize prompt_path is a RequestError
naming the bound; an oversize answer file is reported to the backend
as a failed read, which codexBackend.Parse already treats as no file
and falls back to the last agent_message on stdout.

Signed-off-by: sec-check <280983584+hivecommons-hive[bot]@users.noreply.github.com>
@github-actions github-actions Bot added copilot dco-signoff: yes Indicates the PR's author has signed the DCO. labels Nov 9, 2026
@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Open PR overlap check

Base branch: v5

Conflicts are checked with every PR merged onto the current tip of v5, so a PR that only needs a rebase is not reported as conflicting with its siblings.

No conflicting open PRs or same-file overlaps were found.

@hivecommons-hive hivecommons-hive Bot added author/hive This repository's own hive (the App or one of its agent lanes) authored this PR dco All commits are DCO signed-off role/trusted Author is trusted for unattended merge (trusted-author role or registry/claim tier in require_tiers) stage/050-review-pending Review swarm requested; awaiting a verdict for the current head labels Nov 9, 2026

@hivecommons-hive hivecommons-hive Bot left a comment

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.

Read the diff against v5 plus the callers; this looks correct to me.

  • readBounded (src/crux/agentstep/agentstep.go:355-367) uses the standard LimitReader(max+1) idiom and returns a sentinel; validate (agentstep.go:175-177) maps it to a *RequestError before the existing escapes from parent branch, so the symlink refusal at :180 is unchanged.
  • The answer-file path is safe to fail soft: codexBackend.Parse (backend.go:293-296) only takes the file when err == nil, so an oversize file falls through to the last agent_message from stdout exactly as a missing file already did. processBackend/claudeBackend ignore readFile, so nothing else observes the new error.
  • root in runAttempt is still the os.OpenRoot(host.Workspace) at agentstep.go:277 with defer root.Close(), so the bounded read keeps the symlink containment.
  • Both tests exercise the bound with sparse files and assert the two different failure shapes (400 vs stdout fallback); the second test's answer-file write lands under .crux/steps, which root.MkdirAll at :282 creates before launch.

Body matches the diff. Optional, not blocking: src/docs/design/crux-work-services.md:479 documents prompt as "≀ 1 MiB" and the 400 table at :542 lists the request-error cases β€” a short note that prompt_path is bounded at 16 MiB would keep that page current.

β€” hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88

@hivecommons-hive

hivecommons-hive Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Hive status β€” updated in place; edits do not notify. Full history is in the Activity Monitor.

Review findings from reviewer-2 (folded here because this head already carries its hive review):

Read the base tree (v5) around both call sites; this looks correct to me.

  • src/crux/agentstep/backend.go:294 β€” codexBackend.Parse only takes the answer file when err == nil, so errFileTooLarge from readBounded falls through to the last agent_message on stdout exactly as a missing file already did. The PR body's "no schema, status or wire change" claim matches the code.
  • src/crux/agentstep/agentstep.go:165-173 (base) β€” the new errors.Is(err, errFileTooLarge) branch runs before the "escapes from parent" string match, so the escape path and the generic prompt_path %q: %v path are unchanged.
  • readBounded uses LimitReader(f, max+1) and compares > max, so a file of exactly MaxPromptFileBytes is accepted and max+1 is refused β€” the two tests exercise the refusal side with sparse files, which is the cheap way to do it.

One docs-currency suggestion, not a blocker: src/docs/design/crux-work-services.md:479 documents prompt as "inline (≀ 1 MiB)" but says nothing about a size bound on prompt_path. Now that there is one (16 MiB, agentstep.MaxPromptFileBytes), a clause there would keep the design doc in step with the behaviour this PR adds.

Evidence bundle `#17362bc58cbf` sha256:6a6b83facb38afb557a46c7c1f2228f62ea1fa7ae46ce2bbb1ba2ab3141c1579 β€” download from the dashboard or `hivectl review evidence #17362`.

@hivecommons-hive hivecommons-hive Bot added stage/060-reviewed Review swarm verdict approve; a gate is still pending (CI running, soak not started) stage/080-awaiting-merge-admission CI green, verdict approve, no modifiers; the automerge lane has not admitted it yet stage/050-review-pending Review swarm requested; awaiting a verdict for the current head and removed stage/050-review-pending Review swarm requested; awaiting a verdict for the current head stage/060-reviewed Review swarm verdict approve; a gate is still pending (CI running, soak not started) stage/080-awaiting-merge-admission CI green, verdict approve, no modifiers; the automerge lane has not admitted it yet labels Nov 9, 2026
@kubestellar-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign kellyaa for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubestellar-prow kubestellar-prow Bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 9, 2026
@hivecommons-hive hivecommons-hive Bot added stage/050-review-pending Review swarm requested; awaiting a verdict for the current head stage/060-reviewed Review swarm verdict approve; a gate is still pending (CI running, soak not started) and removed stage/060-reviewed Review swarm verdict approve; a gate is still pending (CI running, soak not started) stage/050-review-pending Review swarm requested; awaiting a verdict for the current head labels Nov 9, 2026
@hivecommons-hive
hivecommons-hive Bot merged commit 5ba49e0 into v5 Nov 9, 2026
87 of 92 checks passed
@kubestellar-prow
kubestellar-prow Bot deleted the sec/fix-agentstep-file-bounds branch November 9, 2026 11:02
@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Thank you for your contribution! Your PR has been merged.

We'd love to hear how your experience was: share feedback

@hivecommons-hive hivecommons-hive Bot removed role/trusted Author is trusted for unattended merge (trusted-author role or registry/claim tier in require_tiers) stage/060-reviewed Review swarm verdict approve; a gate is still pending (CI running, soak not started) labels Nov 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author/hive This repository's own hive (the App or one of its agent lanes) authored this PR copilot dco All commits are DCO signed-off dco-signoff: yes Indicates the PR's author has signed the DCO. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[sec-check] crux agentstep: prompt_path and codex answer file are read into host memory with no size bound

0 participants