mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
* feat(providers): Codex CLI provider — OpenAI via ModelProvider.OPENAI Mirrors the grok blueprint end to end: CodexCliProvider (RO ~/.codex mount, ANTHROPIC_* blanked), an orchestrator-side codex_auth.py refresher (JWT-exp staleness, atomic rewrite, lock-serialized single-use rotation, --check backstop; the CLI's own in-process refresh write no-ops on the RO mount by design — margins keep the orchestrator ahead of the CLI's 5-minute window), config.toml rendering with required=true gateway MCP servers, execpolicy deny rules (forbidden-only), per-role --sandbox (developer=workspace-write, review/doc roles read-only), codex exec --json with pinned ROBOCO_CODEX_CLI_MODEL (gpt-5.3-codex), usage summed from typed turn.completed events priced via the real 4-bucket split, dedicated image + entrypoint, registry/park/finalize/ compose/release wiring. V1 excludes interactive intake/secretary. Per adversarial review: migration 083 seeds the openai provider row enabled=True (without it every routing path 404'd — the whole feature was operationally dead code; grok needed the same seed in 039), the panel picker gained the OpenAI catalog group it silently lacked, and exit classification is structural — only stderr and error.message fields from error events are sniffed (word-boundaried patterns, exact auth phrases, bare 'login' dropped), so the model echoing on-topic words can never false-park the provider fleet-wide, proven by a benign-transcript test. Known open risk flagged, not claimed: whether codex's workspace-write OS sandbox excludes /app is unverified, and no hook mechanism exists to port the bash-guard defense-in-depth. * fix(providers): containment barrier on usage.json reads (code scanning) CodeQL flagged the codex usage read as path injection — correctly: os.path.basename does not neutralize '..', and the upstream segment validator isn't in CodeQL's taint model. The grok/codex reads collapse into one _read_usage_json_contained helper that resolves the built path and refuses anything outside the resolved usage root — a hostile id can never escape regardless of upstream drift. Traversal + containment regression tests added; a stray noqa in the test file replaced with a named constant per repo rule. * fix(providers): use realpath+startswith containment CodeQL recognizes The is_relative_to() guard was a real barrier but not in CodeQL's py/path-injection sanitizer model, so the alert persisted. Switch to the canonical os.path.realpath + startswith(root + os.sep) form, which CodeQL recognizes as a path-traversal barrier; behavior is identical (refuse any candidate resolving outside the usage root). * fix(providers): regexp-allowlist the usage-id segment (CodeQL barrier) Neither is_relative_to nor realpath+startswith was recognized by CodeQL's py/path-injection sanitizer model across the str->Path->open flow. Sanitize the tainted component at the source instead: the id must fullmatch a strict slug token ([A-Za-z0-9][A-Za-z0-9._-]*, no separators, no '..'), which CodeQL recognizes as a path-injection barrier; the realpath+startswith containment stays as defense-in-depth. * fix(providers): standalone regexp guard so CodeQL recognizes the barrier The sanitizer was one disjunct of a compound 'or' condition, which CodeQL's guard analysis does not trace as a barrier. Split the regexp fullmatch into its own single-condition guard (the redundant '..' check is dropped — the required alphanumeric first char already excludes it). --------- Co-authored-by: Renn F <rennf93@users.noreply.github.com>
189 lines
6.7 KiB
Python
189 lines
6.7 KiB
Python
"""codex_cli_sniff — classify a Codex run from ONLY its machine-relevant text.
|
|
|
|
The structural guarantee under test: the model's own on-topic prose (which
|
|
can legitimately contain the words "quota-limited", "login page", or a "429"
|
|
substring inside a commit hash / id) must NEVER reach the classifier, because
|
|
extraction only pulls ``error.message`` fields off error-bearing JSONL events
|
|
plus raw stderr — never ``turn.completed`` / ``item.*`` content.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import json
|
|
from typing import TYPE_CHECKING
|
|
|
|
from roboco.llm.providers import codex_cli_sniff as sniff
|
|
|
|
if TYPE_CHECKING:
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
|
|
def _write_jsonl(path: Path, lines: list[str]) -> None:
|
|
path.parent.mkdir(parents=True, exist_ok=True)
|
|
path.write_text("\n".join(lines) + "\n", encoding="utf-8")
|
|
|
|
|
|
def _turn_failed(message: str) -> str:
|
|
return json.dumps({"type": "turn.failed", "error": {"message": message}})
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# extract_error_text — structural isolation
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def test_extract_error_text_pulls_only_error_message(tmp_path: Path) -> None:
|
|
log = tmp_path / "run.jsonl"
|
|
_write_jsonl(
|
|
log,
|
|
[
|
|
json.dumps(
|
|
{
|
|
"type": "turn.completed",
|
|
"usage": {"input_tokens": 1},
|
|
"text": "the quota-limited rollout ships this sprint",
|
|
}
|
|
),
|
|
_turn_failed("real error text"),
|
|
],
|
|
)
|
|
assert sniff.extract_error_text(log) == "real error text"
|
|
|
|
|
|
def test_extract_error_text_empty_for_missing_or_error_less_log(
|
|
tmp_path: Path,
|
|
) -> None:
|
|
assert sniff.extract_error_text(tmp_path / "nope.jsonl") == ""
|
|
log = tmp_path / "run.jsonl"
|
|
_write_jsonl(log, [json.dumps({"type": "turn.completed", "usage": {}})])
|
|
assert sniff.extract_error_text(log) == ""
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# The false-positive class this fix exists to kill
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def test_benign_transcript_never_false_parks(tmp_path: Path) -> None:
|
|
"""A transcript whose ONLY content is benign on-topic prose — mentioning
|
|
"quota-limited" work, a "login page" bug, and a commit hash containing
|
|
"429" — must classify as "" (no park), because none of it lives in an
|
|
error field the extractor even looks at."""
|
|
log = tmp_path / "run.jsonl"
|
|
_write_jsonl(
|
|
log,
|
|
[
|
|
json.dumps(
|
|
{
|
|
"type": "turn.completed",
|
|
"usage": {"input_tokens": 10, "output_tokens": 5},
|
|
}
|
|
),
|
|
json.dumps(
|
|
{
|
|
"type": "item.completed",
|
|
"item": {
|
|
"type": "agent_message",
|
|
"text": (
|
|
"Fixed the quota-limited rollout gate and the "
|
|
"login page redirect bug. Committed as abc4291f."
|
|
),
|
|
},
|
|
}
|
|
),
|
|
],
|
|
)
|
|
err_log = tmp_path / "run.err"
|
|
err_log.write_text("", encoding="utf-8")
|
|
assert sniff.classify(log, err_log) == ""
|
|
|
|
|
|
def test_word_boundary_prevents_429_substring_false_positive() -> None:
|
|
# "429" embedded inside a larger digit/word run must not match — grok's
|
|
# own \b429\b pattern, restored here after an initial cut dropped it.
|
|
assert not sniff.is_rate_limited("commit abc14293 deployed to prod")
|
|
assert not sniff.is_rate_limited("fix4297abc landed")
|
|
# "quota" alone legitimately matches wherever it appears (grok's own
|
|
# pattern, unchanged) — the false-positive class this fix kills is SCOPE
|
|
# (which text gets scanned, i.e. never turn.completed/item.* content),
|
|
# not the word "quota" itself. See test_benign_transcript_never_false_parks.
|
|
|
|
|
|
def test_bare_login_word_does_not_classify_as_auth() -> None:
|
|
# "login" was dropped from the auth pattern — a mention of a login PAGE
|
|
# (this repo's own panel) must not false-park the provider.
|
|
assert not sniff.is_auth_failure("please visit the login page to continue")
|
|
assert not sniff.is_auth_failure("login required")
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# True positives — real machine-extracted error text
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def test_real_429_error_message_classifies_rate_limit(tmp_path: Path) -> None:
|
|
log = tmp_path / "run.jsonl"
|
|
_write_jsonl(log, [_turn_failed("Rate limit exceeded: 429 Too Many Requests")])
|
|
assert sniff.classify(log) == "rate_limit"
|
|
|
|
|
|
def test_insufficient_quota_error_message_classifies_rate_limit(
|
|
tmp_path: Path,
|
|
) -> None:
|
|
log = tmp_path / "run.jsonl"
|
|
_write_jsonl(log, [_turn_failed("insufficient_quota: billing hard limit hit")])
|
|
assert sniff.classify(log) == "rate_limit"
|
|
|
|
|
|
def test_exact_auth_phrase_refresh_token_expired_classifies_auth(
|
|
tmp_path: Path,
|
|
) -> None:
|
|
log = tmp_path / "run.jsonl"
|
|
_write_jsonl(
|
|
log, [_turn_failed("Your refresh token has expired, please re-authenticate")]
|
|
)
|
|
assert sniff.classify(log) == "auth"
|
|
|
|
|
|
def test_exact_auth_phrase_not_signed_in_classifies_auth(tmp_path: Path) -> None:
|
|
log = tmp_path / "run.jsonl"
|
|
_write_jsonl(log, [_turn_failed("Error: not signed in")])
|
|
assert sniff.classify(log) == "auth"
|
|
|
|
|
|
def test_classify_reads_stderr_too(tmp_path: Path) -> None:
|
|
log = tmp_path / "run.jsonl"
|
|
_write_jsonl(log, [json.dumps({"type": "turn.completed", "usage": {}})])
|
|
err_log = tmp_path / "run.err"
|
|
err_log.write_text("fatal: 429 too many requests\n", encoding="utf-8")
|
|
assert sniff.classify(log, err_log) == "rate_limit"
|
|
|
|
|
|
def test_classify_missing_files_returns_empty(tmp_path: Path) -> None:
|
|
assert sniff.classify(tmp_path / "nope.jsonl", tmp_path / "nope.err") == ""
|
|
|
|
|
|
def test_rate_limit_checked_before_auth_when_both_present(tmp_path: Path) -> None:
|
|
log = tmp_path / "run.jsonl"
|
|
_write_jsonl(
|
|
log,
|
|
[_turn_failed("429 too many requests, and also not signed in downstream")],
|
|
)
|
|
assert sniff.classify(log) == "rate_limit"
|
|
|
|
|
|
def test_main_cli_prints_classification(
|
|
tmp_path: Path, capsys: pytest.CaptureFixture[str]
|
|
) -> None:
|
|
log = tmp_path / "run.jsonl"
|
|
_write_jsonl(log, [_turn_failed("429 too many requests")])
|
|
assert sniff.main([str(log)]) == 0
|
|
assert capsys.readouterr().out.strip() == "rate_limit"
|
|
|
|
|
|
def test_main_cli_no_args_prints_empty(capsys: pytest.CaptureFixture[str]) -> None:
|
|
assert sniff.main([]) == 0
|
|
assert capsys.readouterr().out.strip() == ""
|