diff --git a/README.md b/README.md index 0fe4b5f8..fea7e528 100644 --- a/README.md +++ b/README.md @@ -156,8 +156,9 @@ branch before the agent starts, blocking both startup-time code execution and prompt injection from a PR's own copy of those files. **Rate limiting** — Burst detection (10 PRs and 10 issues per 20 minutes, -checked independently) and daily spike detection halt the bot before runaway -loops cause damage. +checked independently) and a hard daily ceiling halt the bot before runaway +loops cause damage; a lower daily spike threshold stops it opening new issues +and PRs without stopping the rest of its work. **Fixed prompts** — Workflow prompts come from the action, not from attacker-controlled input like PR descriptions or comments. diff --git a/codex/action.yaml b/codex/action.yaml index c42895f0..b5eb4569 100644 --- a/codex/action.yaml +++ b/codex/action.yaml @@ -170,6 +170,12 @@ runs: envsubst '$BOT_NAME' < "$SHARED" echo envsubst '$BOT_NAME' < "$TAIL" + # Set by rate-limit-preflight.sh on a spike-tier trip; the Claude + # harness appends the same text in compose-system-prompt.sh. + if [ -n "${TEND_CREATION_PAUSED_NOTE:-}" ]; then + echo + echo "$TEND_CREATION_PAUSED_NOTE" + fi } > "$AGENTS" echo "Staged AGENTS.md at $AGENTS ($(wc -l < "$AGENTS") lines)" @@ -246,6 +252,13 @@ runs: EFFORT: ${{ inputs.effort }} SANDBOX: ${{ inputs.sandbox }} run: | + # Skill-output handoff read by "Append skill step summary" below. The + # Claude harness creates this as the sandbox user in setup-sandbox.sh; + # here the agent runs as the runner user, so a plain mkdir suffices. + # Deterministic rather than left to the agent: Codex writes files + # through the shell, where a missing parent is a hard failure. + mkdir -p /tmp/claude + OUTPUT_FILE="$RUNNER_TEMP/codex-final-message.md" ARGS=( --model "$MODEL" @@ -272,6 +285,19 @@ runs: fi exit $EXIT + # Same handoff as the Claude harness: skills that have no thread to post to + # (review-runs, review-reviewers, and any run under a creation pause) write + # their result here and it lands on the run page. Without this step those + # writes go nowhere under Codex. + - name: Append skill step summary + if: always() + shell: bash + run: | + if [ -f /tmp/claude/step-summary.md ]; then + cat /tmp/claude/step-summary.md >> "$GITHUB_STEP_SUMMARY" + echo "" >> "$GITHUB_STEP_SUMMARY" + fi + # Codex writes rollouts to ~/.codex/sessions/YYYY/MM/DD/rollout-*.jsonl. # Each event includes a token_count field (input/output/cached). Sum # them. Cost is left at 0 — Codex CLI doesn't surface API list prices diff --git a/docs/security-model.md b/docs/security-model.md index 5f4d8dc4..c24db604 100644 --- a/docs/security-model.md +++ b/docs/security-model.md @@ -318,11 +318,20 @@ Codex harness (`codex/action.yaml`) still passes both the PAT and the model auth directly to the agent. The merge restriction and the environment gate remain the load-bearing boundaries regardless of harness. -**Rate limiting.** Burst detection (10 PRs or issues per 20 minutes) and -spike detection (today's volume vs 6-day baseline, scaled per repo) abort -the run before Claude starts, catching runaway loops between workflows. -The check runs as a shell step, so a prompt-injection attack inside the -Claude session cannot skip it. Concrete limits live in +**Rate limiting.** Burst detection (10 PRs or issues per 20 minutes) and a +hard daily ceiling (a 6-day baseline's worth of issues and PRs in one day, +and never less than 10 above the spike threshold so a baseline depressed by +a quiet week can't collapse the two together) abort the run before Claude +starts, catching runaway loops between workflows. A softer daily spike +threshold (today's volume vs the same baseline, scaled per repo) does not +abort: the run proceeds with a creation-pause directive appended to the +agent's prompt, so it keeps reviewing, replying, and pushing while opening +no new issues or PRs. A run with no triggering thread is told to write what +it would have filed to the job summary rather than drop it. The +thresholds are computed in a shell step, so a prompt-injection attack +inside the Claude session cannot skip the two abort tiers — it could +disregard the spike tier's prompt directive, which is why that tier sits +below a hard ceiling rather than replacing one. Concrete limits live in `shared/steps/rate-limit-preflight.sh`. **Fixed prompts and marketplace skills.** The prompt and skill set come from diff --git a/generator/tests/test_shared_steps.py b/generator/tests/test_shared_steps.py index 1814d22b..debbafa0 100644 --- a/generator/tests/test_shared_steps.py +++ b/generator/tests/test_shared_steps.py @@ -146,3 +146,202 @@ def test_mark_notification_read_leaves_activity_newer_than_the_run( assert result.returncode == 0, result.stderr assert not any("-X PATCH" in c for c in _calls(gh_env)) + + +# --- rate-limit-preflight.sh ------------------------------------------------- + +RATE_LIMIT_PREFLIGHT = REPO_ROOT / "shared" / "steps" / "rate-limit-preflight.sh" + +# `gh api` stand-in returning the four counts the preflight reads. The two +# search/issues calls are told apart by the `..` date range only the baseline +# query carries. FAKE_ prefixes keep these clear of the names the script +# assigns, so a test can't pass on an inherited value it never fetched. +FAKE_GH_COUNTS = """#!/usr/bin/env bash +case "$2" in + repos/*/pulls*) echo "$FAKE_BURST_PRS" ;; + repos/*/issues*) echo "$FAKE_BURST_ISSUES" ;; + *created:*..*) echo "$FAKE_BASELINE" ;; + search/issues*) echo "$FAKE_TODAY" ;; + *) exit 1 ;; +esac +""" + +# A 6-day baseline of 17 is what the bot actually carried into 2026-08-05: it +# puts the spike tier at 10 + 17/3 = 15 and the hard tier at 10 + 17 = 27. +BASELINE = 17 + + +def _preflight( + tmp_path: Path, + *, + today: int, + baseline: int = BASELINE, + burst_prs: int = 0, + burst_issues: int = 0, +) -> tuple[subprocess.CompletedProcess[str], str]: + """Run the preflight against fixed counts; return the result and GITHUB_ENV.""" + bindir = tmp_path / "fakebin" + bindir.mkdir() + gh = bindir / "gh" + gh.write_text(FAKE_GH_COUNTS) + gh.chmod(0o755) + github_env = tmp_path / "github-env" + github_env.write_text("") + + result = subprocess.run( + ["bash", str(RATE_LIMIT_PREFLIGHT)], + env={ + "PATH": f"{bindir}:/usr/bin:/bin", + "GITHUB_REPOSITORY": "owner/repo", + "GITHUB_ENV": str(github_env), + "BOT_NAME": "bot", + "FAKE_TODAY": str(today), + "FAKE_BASELINE": str(baseline), + "FAKE_BURST_PRS": str(burst_prs), + "FAKE_BURST_ISSUES": str(burst_issues), + }, + capture_output=True, + text=True, + ) + return result, github_env.read_text() + + +def test_rate_limit_spike_tier_pauses_creation_without_aborting( + tmp_path: Path, +) -> None: + """The spike tier lets the run proceed and pauses creation instead. + + Aborting here took every workflow on the repo down for the rest of the UTC + day — `tend-review` and `tend-mention` included, which answer humans and + create nothing, so they cannot contribute to the count being enforced. This + is the 2026-08-05 case exactly: 16 items against a spike limit of 15. + """ + result, github_env = _preflight(tmp_path, today=16) + + assert result.returncode == 0, ( + f"spike tier aborted the run (exit {result.returncode}); " + f"stdout:\n{result.stdout}\nstderr:\n{result.stderr}" + ) + assert "TEND_CREATION_PAUSED_NOTE" in github_env, ( + f"no creation pause exported; GITHUB_ENV:\n{github_env}" + ) + assert "do not open a new issue or pull request" in github_env + + +def test_rate_limit_hard_tier_still_aborts(tmp_path: Path) -> None: + """A slow runaway crosses the hard tier and is stopped deterministically. + + The spike tier is advisory once it reaches the model, so this is the gate a + prompt-injected session cannot talk its way past. + """ + result, github_env = _preflight(tmp_path, today=28) + + assert result.returncode != 0, ( + f"runaway volume did not abort the run; stdout:\n{result.stdout}" + ) + assert "hard limit of 27" in result.stdout + assert "TEND_CREATION_PAUSED_NOTE" not in github_env, ( + "exported a pause note on a run that aborts anyway" + ) + + +def test_rate_limit_under_spike_tier_is_a_clean_pass(tmp_path: Path) -> None: + """Normal volume neither aborts nor pauses creation.""" + result, github_env = _preflight(tmp_path, today=3) + + assert result.returncode == 0, result.stderr + assert "Rate limit check passed" in result.stdout + assert "TEND_CREATION_PAUSED_NOTE" not in github_env + + +def test_rate_limit_burst_aborts_regardless_of_daily_volume(tmp_path: Path) -> None: + """The 20-minute burst check is untouched: it still aborts the run.""" + result, github_env = _preflight(tmp_path, today=3, burst_prs=11) + + assert result.returncode != 0, ( + f"burst of 11 PRs in 20 minutes did not abort; stdout:\n{result.stdout}" + ) + assert "TEND_CREATION_PAUSED_NOTE" not in github_env + + +def test_rate_limit_pause_tier_survives_a_zero_baseline(tmp_path: Path) -> None: + """A baseline of 0 still leaves a pause band between the two tiers. + + Both limits scale off the baseline (`10 + P` and `10 + P/3`), so they + converge as it falls and coincide at 10 when it reaches 0 — which would put + the 11th item straight into the hard tier and revert this script to the + behaviour it exists to replace. A baseline of 0 is not exotic: a quota + outage, a quiet week, or an adopter's install day all produce one, and a + depressed baseline is the diagnosed trigger. + """ + result, github_env = _preflight(tmp_path, today=11, baseline=0) + + assert result.returncode == 0, ( + f"zero baseline collapsed the tiers and aborted; stdout:\n{result.stdout}" + ) + assert "TEND_CREATION_PAUSED_NOTE" in github_env, ( + f"no creation pause exported; GITHUB_ENV:\n{github_env}" + ) + assert "hard: 20" in result.stdout, ( + f"hard limit not floored above the spike limit; stdout:\n{result.stdout}" + ) + + +def test_rate_limit_burst_abort_exports_no_pause_note(tmp_path: Path) -> None: + """A burst abort that also lands in the spike band exports no directive. + + The two conditions are independent, so a run can be over the daily spike + limit *and* bursting. It exits 1 either way; a creation directive addressed + to an agent that never starts is dead state. + """ + result, github_env = _preflight(tmp_path, today=16, burst_issues=11) + + assert result.returncode != 0, ( + f"burst of 11 issues in 20 minutes did not abort; stdout:\n{result.stdout}" + ) + assert "TEND_CREATION_PAUSED_NOTE" not in github_env, ( + "exported a pause note on a run that aborts anyway" + ) + + +def test_skill_summary_dir_is_created_by_both_harnesses() -> None: + """Both harnesses create `/tmp/claude` before the agent runs. + + It is the only channel out of the session for a run with no thread to post + to: the sandbox is denylisted from `$GITHUB_STEP_SUMMARY` itself, so the + agent writes a file and a teardown step copies it. Nothing creates the + directory implicitly — `/tmp` is `1777`, and Codex writes files through the + shell, where a missing parent is a hard failure rather than an auto-mkdir. + Leaving it to the agent would make the copy step read an absent file on + exactly the paused runs it exists for. + """ + claude = (REPO_ROOT / "proxy" / "setup-sandbox.sh").read_text() + codex = (REPO_ROOT / "codex" / "action.yaml").read_text() + + assert 'sudo -u "$SANDBOX" mkdir -p /tmp/claude' in claude, ( + "setup-sandbox.sh must create /tmp/claude as the sandbox user; a " + "runner-owned directory under 1777 /tmp is unwritable by the agent" + ) + assert "mkdir -p /tmp/claude" in codex, ( + "codex/action.yaml must create /tmp/claude before running the agent" + ) + for name in ("claude/action.yaml", "codex/action.yaml"): + assert "/tmp/claude/step-summary.md" in (REPO_ROOT / name).read_text(), ( + f"{name} no longer copies the skill summary into the job summary" + ) + + +def test_rate_limit_pause_note_covers_runs_with_no_thread(tmp_path: Path) -> None: + """The pause directive names a destination for threadless runs. + + The workflows whose entire deliverable is a new issue or PR are the + scheduled and `workflow_run` ones, which have no triggering thread to defer + to. Without an explicit destination the pause converts a loud failure into a + silent one: the agent does the diagnosis, finds nowhere to put it, and the + result dies with the runner. + """ + _, github_env = _preflight(tmp_path, today=16) + + assert "/tmp/claude/step-summary.md" in github_env, ( + f"pause note leaves threadless runs without a destination:\n{github_env}" + ) diff --git a/proxy/setup-sandbox.sh b/proxy/setup-sandbox.sh index 815d21ce..a9e0e52e 100755 --- a/proxy/setup-sandbox.sh +++ b/proxy/setup-sandbox.sh @@ -300,6 +300,14 @@ log "workspace handed to $SANDBOX" sudo -u "$SANDBOX" mkdir -p "$TEND_RUN_DIR" log "run dir $TEND_RUN_DIR" +# Skill-output handoff: the agent writes step-summary.md here and the action's +# teardown copies it into $GITHUB_STEP_SUMMARY (the sandbox is denylisted from +# GITHUB_STEP_SUMMARY itself, so a file is the only channel). Created as the +# sandbox user — /tmp is 1777, so a runner-owned dir here would be unwritable +# by the agent. Fixed path because the skills that use it hard-code it. +sudo -u "$SANDBOX" mkdir -p /tmp/claude +log "skill summary dir /tmp/claude" + # 4. Start the injecting proxy. It inherits the real GitHub + Anthropic # credentials from this shell; they never leave this runner-owned process. # confdir is 0700 runner-only so the sandbox can't read the CA private key diff --git a/shared/steps/compose-system-prompt.sh b/shared/steps/compose-system-prompt.sh index 58e25e88..df442542 100755 --- a/shared/steps/compose-system-prompt.sh +++ b/shared/steps/compose-system-prompt.sh @@ -11,7 +11,9 @@ # # Inputs (env): SYSTEM_PROMPT_FILE (absolute path to shared/system-prompt.md; # differs per action by checkout depth), BOT_NAME, EXTRA (system_prompt_append), -# GITHUB_OUTPUT (from Actions). +# GITHUB_OUTPUT (from Actions), TEND_CREATION_PAUSED_NOTE (reaches this step +# through GITHUB_ENV when rate-limit-preflight.sh trips its spike tier; empty +# otherwise). The pause note goes last so it outranks the adopter's append. set -eo pipefail SHARED="$SYSTEM_PROMPT_FILE" @@ -22,6 +24,9 @@ FULL="${CLAUDE_DIRECTIVE}"$'\n\n'"${AUTONOMY_DIRECTIVE}"$'\n\n'"${BASE}" if [ -n "$EXTRA" ]; then FULL="${FULL}"$'\n\n'"${EXTRA}" fi +if [ -n "${TEND_CREATION_PAUSED_NOTE:-}" ]; then + FULL="${FULL}"$'\n\n'"${TEND_CREATION_PAUSED_NOTE}" +fi { echo 'value<10 PRs or >10 issues in 20 minutes) -> abort: only a loop does this +# today > hard limit (a 6-day baseline's worth +# of output in one day) -> abort: slow runaway +# today > spike limit -> creation pause, run continues +# +# The spike tier used to abort too. It is a *creation* limit, so aborting took +# down work that creates nothing — reviews, mention replies, triage comments, +# CI fixes — for the rest of the UTC day, and a busy day after a quiet week +# (which depresses the baseline) trips it. Instead the run proceeds with +# TEND_CREATION_PAUSED_NOTE in the environment; both actions append it to the +# agent's prompt, so the agent still does its comment/review/push work and +# leaves new issues and PRs alone. The two abort tiers keep a hard stop that no +# prompt-injected session can talk its way past. # # Inputs (env): GITHUB_TOKEN (for gh), BOT_NAME (bot username), # GITHUB_REPOSITORY (from Actions). +# Outputs (env, via GITHUB_ENV): TEND_CREATION_PAUSED_NOTE, set only on a +# spike-tier trip. set -eo pipefail REPO="${GITHUB_REPOSITORY}" @@ -29,8 +45,23 @@ PAST_POSTS=$(gh api "search/issues?q=author:${BOT}+repo:${REPO}+created:${SIX_DA --jq '.total_count' || echo 0) # spike_limit = 10 + 2 * daily_avg = 10 + 2 * (past_posts / 6) = 10 + past_posts / 3 SPIKE_LIMIT=$((10 + PAST_POSTS / 3)) +# hard_limit = 10 + past_posts: a whole baseline week's output in a single day. +# No productive day reaches it; a loop does. +# +# Both limits scale off PAST_POSTS, so they converge as the baseline falls: at +# a baseline of 0 they are both 10 and the pause tier is unreachable — the run +# hard-aborts on the 11th item, exactly the behaviour this script moved away +# from. A depressed baseline is the failure mode being fixed (a quota outage, a +# quiet week, an adopter's install day), so the pause tier has to survive it. +# Floor the gap at 10, matching the burst limit and the constant term in both +# formulas. It only binds below a baseline of ~15; above that the formula +# already opens a wider band. +HARD_LIMIT=$((10 + PAST_POSTS)) +if [ "$HARD_LIMIT" -lt $((SPIKE_LIMIT + 10)) ]; then + HARD_LIMIT=$((SPIKE_LIMIT + 10)) +fi -echo "Rate limit: burst=${RECENT_PRS} PRs, ${RECENT_ISSUES} issues (20min); today=${TODAY_POSTS} (limit: ${SPIKE_LIMIT})" +echo "Rate limit: burst=${RECENT_PRS} PRs, ${RECENT_ISSUES} issues (20min); today=${TODAY_POSTS} (spike: ${SPIKE_LIMIT}, hard: ${HARD_LIMIT})" ABORT=false if [ "$RECENT_PRS" -gt 10 ]; then @@ -41,9 +72,21 @@ if [ "$RECENT_ISSUES" -gt 10 ]; then echo "::error::Rate limit: bot created ${RECENT_ISSUES} issues in the last 20 minutes (limit: 10)" ABORT=true fi -if [ "$TODAY_POSTS" -gt "$SPIKE_LIMIT" ]; then - echo "::error::Rate limit: bot created ${TODAY_POSTS} items today, above spike limit of ${SPIKE_LIMIT} (baseline: ${PAST_POSTS} over past 6 days)" +if [ "$TODAY_POSTS" -gt "$HARD_LIMIT" ]; then + echo "::error::Rate limit: bot created ${TODAY_POSTS} items today, above hard limit of ${HARD_LIMIT} (baseline: ${PAST_POSTS} over past 6 days)" ABORT=true +elif [ "$TODAY_POSTS" -gt "$SPIKE_LIMIT" ]; then + echo "::warning::Creation pause: bot created ${TODAY_POSTS} items today, above spike limit of ${SPIKE_LIMIT} (baseline: ${PAST_POSTS} over past 6 days). The run continues; opening new issues and PRs is off-limits for it." + # Skip the note when a burst check already set ABORT: the run exits 1 below, + # so exporting a creation directive for an agent that never starts is dead + # state that contradicts the invariant the abort tiers otherwise hold. + if [ "$ABORT" = false ] && [ -n "${GITHUB_ENV:-}" ]; then + { + echo 'TEND_CREATION_PAUSED_NOTE<> "$GITHUB_ENV" + fi fi if [ "$ABORT" = true ]; then exit 1