fix: close reviewed tool identity and credential redaction edge cases
This commit is contained in:
1 parent
54763631e9
commit
72734b7371
8 files changed
+36
-13
No files matched your search
@@ -473,9 +473,10 @@ class RailRunDashboard:
|
||||
for step in self.steps:
|
||||
if step.call_id == raw_call_id:
|
||||
return step
|
||||
for step in reversed(self.steps):
|
||||
if step.status == "active" and (not raw_tool or step.tool == raw_tool):
|
||||
return step
|
||||
else:
|
||||
for step in reversed(self.steps):
|
||||
if step.status == "active" and (not raw_tool or step.tool == raw_tool):
|
||||
return step
|
||||
step = RailStep(title=_tool_title(raw_tool or "tool", None), tool=raw_tool, call_id=raw_call_id)
|
||||
self.steps.append(step)
|
||||
self.steps = self.steps[-10:]
|
||||
|
||||
@@ -510,11 +510,16 @@ _TEXT_PRIVATE_KEY_PATTERN = re.compile(
|
||||
#: both leaked it. The value runs to the end of the line or the closing quote of
|
||||
#: a quoted shell argument. An already-redacted value is skipped, which keeps the
|
||||
#: substitution idempotent.
|
||||
_TEXT_QUOTED_COOKIE_HEADER_PATTERN = re.compile(
|
||||
r"(?P<quote>[\"'])(?P<head>(?:set-)?cookie[ \t]*:)(?P<sp>[ \t]*)"
|
||||
r"(?:(?:\\.)|(?!(?P=quote))[^\r\n\\])*(?P=quote)",
|
||||
re.IGNORECASE,
|
||||
)
|
||||
_TEXT_COOKIE_HEADER_PATTERN = re.compile(
|
||||
r"(?P<head>(?<![\w-])(?:set-)?cookie\s*:)"
|
||||
r"(?P<sp>[ \t]*)(?![ \t])"
|
||||
r"(?!\[redacted\])"
|
||||
r"[^\r\n\"']+",
|
||||
r"[^\r\n]+",
|
||||
re.IGNORECASE,
|
||||
)
|
||||
|
||||
@@ -589,6 +594,9 @@ def redact_text(text: object) -> str:
|
||||
if not s:
|
||||
return s
|
||||
s = _TEXT_PRIVATE_KEY_PATTERN.sub(_REDACTED, s)
|
||||
s = _TEXT_QUOTED_COOKIE_HEADER_PATTERN.sub(
|
||||
r"\g<quote>\g<head>\g<sp>" + _REDACTED + r"\g<quote>", s,
|
||||
)
|
||||
s = _TEXT_COOKIE_HEADER_PATTERN.sub(r"\g<head>\g<sp>" + _REDACTED, s)
|
||||
s = _TEXT_BEARER_PATTERN.sub(r"\g<scheme> " + _REDACTED, s)
|
||||
s = _TEXT_AUTH_SCHEME_PATTERN.sub(r"\g<head>\g<scheme> " + _REDACTED, s)
|
||||
|
||||
@@ -69,7 +69,7 @@ def _with_forced_frontmatter_name(content: str, slug: str) -> str:
|
||||
new_lines = []
|
||||
replaced = False
|
||||
for line in header_lines:
|
||||
if line.strip().startswith("name:"):
|
||||
if line.partition(":")[0].strip() == "name":
|
||||
new_lines.append(f"name: {slug}")
|
||||
replaced = True
|
||||
else:
|
||||
|
||||
@@ -26,11 +26,7 @@ _CURRENCY_TOKEN_RE = re.compile(
|
||||
|
||||
# ISO codes of the fiat currencies a broker export can quote as a pair
|
||||
# (EUR/USD); a pair with two fiat legs is forex, never crypto.
|
||||
_FIAT_CODES = frozenset({
|
||||
"USD", "EUR", "GBP", "JPY", "CHF", "AUD", "CAD", "NZD", "CNY", "CNH", "HKD",
|
||||
"SGD", "SEK", "NOK", "DKK", "MXN", "ZAR", "INR", "KRW", "TRY", "PLN", "CZK",
|
||||
"HUF", "ILS", "THB",
|
||||
})
|
||||
from src.market_data import FIAT_CODES as _FIAT_CODES
|
||||
|
||||
FormatName = str # "tonghuashun" | "eastmoney" | "futu" | "generic" | "unknown"
|
||||
|
||||
|
||||
@@ -28,6 +28,12 @@ class TestConcurrentToolCallPairing:
|
||||
it actually belongs to.
|
||||
"""
|
||||
|
||||
def test_missing_call_does_not_complete_another_active_call(self, dashboard):
|
||||
dashboard.handle_event("tool_call", {"tool": "backtest", "call_id": "active"})
|
||||
dashboard.handle_event("tool_result", {"tool": "backtest", "call_id": "evicted", "status": "error"})
|
||||
assert next(step for step in dashboard.steps if step.call_id == "active").status == "active"
|
||||
assert next(step for step in dashboard.steps if step.call_id == "evicted").status == "error"
|
||||
|
||||
def test_results_pair_with_their_own_call_via_call_id(self, dashboard: RailRunDashboard) -> None:
|
||||
dashboard.handle_event(
|
||||
"tool_call", {"tool": "get_market_data", "arguments": {"symbol": "AAPL"}, "call_id": "call-1"}
|
||||
|
||||
@@ -19,6 +19,17 @@ def test_only_the_header_line_is_redacted() -> None:
|
||||
assert out == "HTTP/1.1 200 OK\nSet-Cookie: [redacted]\nContent-Type: text/html"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("text", [
|
||||
'Cookie: sid="secret"; csrf=another',
|
||||
'Set-Cookie: sid="secret"; Path=/',
|
||||
'''curl -H 'Cookie: sid="secret"; csrf=another' https://example.com''',
|
||||
])
|
||||
def test_quoted_cookie_value_never_leaks(text):
|
||||
out = redact_text(text)
|
||||
assert "secret" not in out and "another" not in out
|
||||
assert redact_text(out) == out
|
||||
|
||||
|
||||
@pytest.mark.parametrize("quote", ['"', "'"])
|
||||
def test_quoted_curl_header_keeps_the_rest_of_the_command(quote: str) -> None:
|
||||
cmd = f"curl -H {quote}Cookie: sid=abc123; csrf=xyz{quote} https://example.com/api"
|
||||
|
||||
@@ -87,12 +87,13 @@ class TestSaveSkillTool:
|
||||
text = (user_dir / "custom" / "SKILL.md").read_text(encoding="utf-8")
|
||||
assert "description: Custom skill" in text
|
||||
|
||||
def test_save_forces_frontmatter_name_to_match_slug(self, tool: SaveSkillTool, user_dir: Path) -> None:
|
||||
@pytest.mark.parametrize("field", ["name", "name "])
|
||||
def test_save_forces_frontmatter_name_to_match_slug(self, tool: SaveSkillTool, user_dir: Path, field: str) -> None:
|
||||
"""A frontmatter name that disagrees with the slug this tool advertises
|
||||
would make the skill unreachable under load_skill(slug) and cause
|
||||
duplicate entries on every lookup; the saved name must always match.
|
||||
"""
|
||||
content = "---\nname: MySkill\ndescription: mismatched name\ncategory: user\n---\n\nBody"
|
||||
content = f"---\n{field}: MySkill\ndescription: mismatched name\ncategory: user\n---\n\nBody"
|
||||
result = json.loads(tool.execute(name="My Skill", content=content))
|
||||
assert result["status"] == "ok"
|
||||
text = (user_dir / "my-skill" / "SKILL.md").read_text(encoding="utf-8")
|
||||
|
||||
@@ -11,7 +11,7 @@ from src.tools.trade_journal_parsers import _infer_market_from_symbol, parse_gen
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"symbol",
|
||||
["EUR/USD", "GBP/USD", "USD/JPY", "usd-cnh", "AUD-USD", "USD/CHF"],
|
||||
["EUR/USD", "GBP/USD", "USD/JPY", "usd-cnh", "AUD-USD", "USD/CHF", "USD/BRL", "USD/TWD"],
|
||||
)
|
||||
def test_fiat_cross_is_not_crypto(symbol: str) -> None:
|
||||
assert _infer_market_from_symbol(symbol) == "other"
|
||||
|
||||
Reference in new issue
Block a user