Skip to content

fix(sparc-service): strip agent-injected keys before SPARC evaluates tool calls #738

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
OsherElhadad merged 3 commits into from
Aug 9, 2026
+49 −0
Merged

fix(sparc-service): strip agent-injected keys before SPARC evaluates tool calls #738

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8e4332e
feat(sparc-service): add SPARC_LOG_REQUESTS and SPARC_STRIP_TOOL_ARG_…
vz-ibm Jul 19, 2026
651bb04
fix(sparc-service): configure root logger so application log.info() o…
vz-ibm Jul 19, 2026
ec90760
fix(sparc-service): address PR #738 review findings — Settings integr…
vz-ibm Aug 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions authbridge/sparc-service/sparc_service/__main__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@

from __future__ import annotations

import logging

import uvicorn

from .settings import Settings


def main() -> None:
logging.basicConfig(level=logging.INFO)
settings = Settings.from_env()
uvicorn.run("sparc_service.api:app", host=settings.host, port=settings.port, log_level="info")

Expand Down
34 changes: 34 additions & 0 deletions authbridge/sparc-service/sparc_service/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from __future__ import annotations

import json
import logging

from fastapi import FastAPI, HTTPException
Expand All @@ -19,6 +20,29 @@

log = logging.getLogger(__name__)

# _LOG_REQUESTS and _STRIP_KEYS are now read from Settings (via Settings.from_env)
# so all config comes from a single place. See settings.py.


def _strip_tool_arg_keys(tool_calls: list[dict], keys: frozenset[str]) -> list[dict]:
"""Return a copy of tool_calls with the named argument keys removed."""
result = []
for tc in tool_calls:
fn = tc.get("function") or {}
if not isinstance(fn, dict):
result.append(tc)
continue
raw_args = fn.get("arguments", "")
try:
args = json.loads(raw_args) if isinstance(raw_args, str) else raw_args
if isinstance(args, dict):
args = {k: v for k, v in args.items() if k not in keys}
new_args = json.dumps(args) if isinstance(args, dict) else raw_args
except (json.JSONDecodeError, TypeError):
new_args = raw_args
result.append({**tc, "function": {**fn, "arguments": new_args}})
return result


def create_app(engine: ReflectionEngine | None = None) -> FastAPI:
"""Build the FastAPI app. Inject ``engine`` in tests; defaults to env config."""
Expand Down Expand Up @@ -51,6 +75,16 @@ def readyz() -> dict[str, object]:

@app.post("/reflect", response_model=ReflectResponse)
async def reflect(request: ReflectRequest) -> ReflectResponse:
if settings.log_requests:
log.info("incoming reflect request: %s", request.model_dump_json())

if settings.strip_tool_arg_keys and request.tool_calls:
request = request.model_copy(
update={"tool_calls": _strip_tool_arg_keys(request.tool_calls, settings.strip_tool_arg_keys)}
Comment on lines +81 to +83

@coderabbitai coderabbitai Bot Aug 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 8 \
  'SPARC_STRIP_TOOL_ARG_KEYS|strip_tool_arg_keys|session_id|IBAC|token|authorization' \
  authbridge/sparc-service \
  --glob '*.py' \
  --glob '*.yaml'

Repository: rossoctl/cortex

Length of output: 16840

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant implementations and nearby tests without running repository code.
sed -n '1,130p' authbridge/sparc-service/sparc_service/api.py
printf '\n--- settings.py relevant section ---\n'
sed -n '130,195p' authbridge/sparc-service/sparc_service/settings.py
printf '\n--- tests around strip/logging ---\n'
sed -n '1,180p' authbridge/sparc-service/tests/test_api.py
printf '\n--- config/docs references ---\n'
rg -n -C 3 'SPARC_STRIP_TOOL_ARG_KEYS|strip_tool_arg_keys|SECURE|SESSION|IBAC|Authorization|Authorization|session_id' . \
  --glob '*.md' --glob '*.txt' --glob '*.yaml' --glob '*.yml' --glob '*.py' | sed -n '1,240p'

Repository: rossoctl/cortex

Length of output: 40077

Restrict stripping to harmless request metadata.

The strip setting accepts operator-provided key names, and _strip_tool_arg_keys() removes them from reflection arguments before SPARC evaluates the tool call. If this list includes authorization/session keys used by IBAC, token exchange, or policy decisions, it can bypass controls. Keep /reflect request headers intact, and only strip request-specific logging metadata through a protected allowlist or fixed internal keys, plus a regression test.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@authbridge/sparc-service/sparc_service/api.py` around lines 81 - 83, Restrict
the stripping performed by the request handling flow around _strip_tool_arg_keys
to a protected allowlist of harmless internal logging metadata, rather than
arbitrary operator-provided keys. Preserve /reflect request headers and all
authorization, session, IBAC, token-exchange, and policy-related arguments; add
a regression test proving those keys remain intact while approved metadata is
removed.

Source: Coding guidelines

)
if settings.log_requests:
log.info("after strip (%s): tool_calls=%s", sorted(settings.strip_tool_arg_keys), request.tool_calls)

Comment on lines +78 to +87

@coderabbitai coderabbitai Bot Aug 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Do not log raw request payloads at INFO level.

log.info logs the complete request before stripping and logs tool calls again afterward. With SPARC_LOG_REQUESTS enabled, session identifiers and other tool arguments can enter normal production logs. Use log.debug, redact sensitive configured keys, and ensure the setting enables DEBUG output without promoting payloads to INFO.

As stated in the PR objectives: SPARC_LOG_REQUESTS must log incoming requests at DEBUG level.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@authbridge/sparc-service/sparc_service/api.py` around lines 78 - 87, Update
the request logging in the visible reflect-request handling block so both
payload logs use log.debug rather than log.info, while preserving the existing
SPARC_LOG_REQUESTS setting as the gate for DEBUG output. Redact configured
sensitive tool-argument keys before logging the incoming request, and ensure
neither the pre-strip request nor post-strip tool_calls exposes raw sensitive
values.

# SPARCReflectionComponent.process is synchronous (and CPU/IO bound on the
# LLM call); run it off the event loop so the service stays responsive.
try:
Expand Down
12 changes: 12 additions & 0 deletions authbridge/sparc-service/sparc_service/settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ class Settings:
host: str = "0.0.0.0"
port: int = 8090

# Request logging / arg-stripping (see api.py).
log_requests: bool = False
strip_tool_arg_keys: frozenset = field(default_factory=frozenset)

# Validation errors collected at load time (provider creds missing, etc.).
errors: tuple[str, ...] = field(default_factory=tuple)

Expand Down Expand Up @@ -152,6 +156,12 @@ def from_env(cls) -> "Settings":
f"provider={provider} requires SPARC_MODEL (e.g. azure/<deployment> or anthropic/claude-3-5-sonnet)"
)

strip_tool_arg_keys: frozenset[str] = frozenset(
k.strip()
for k in os.getenv("SPARC_STRIP_TOOL_ARG_KEYS", "").split(",")
if k.strip()
)

return cls(
provider=provider,
model=model,
Expand All @@ -171,5 +181,7 @@ def from_env(cls) -> "Settings":
llm_registry_id=os.getenv("SPARC_LLM_REGISTRY_ID", "").strip(),
host=os.getenv("HOST", "0.0.0.0"),
port=_int_env("PORT", 8090),
log_requests=_truthy(os.getenv("SPARC_LOG_REQUESTS", "")),
strip_tool_arg_keys=strip_tool_arg_keys,
Comment on lines +184 to +185

@coderabbitai coderabbitai Bot Aug 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reject invalid SPARC_LOG_REQUESTS values.

_truthy() returns False for every unrecognized value. A typo such as SPARC_LOG_REQUESTS=treu therefore disables request logging without reporting a configuration error. Validate the value and append an error to errors, consistent with the existing settings validation flow.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@authbridge/sparc-service/sparc_service/settings.py` around lines 184 - 185,
Update the SPARC_LOG_REQUESTS handling in the settings construction flow to
validate unrecognized non-empty values instead of silently treating them as
false. When the value is not an accepted boolean representation, append a
descriptive error to the existing errors collection while preserving valid true,
false, and unset behavior.

errors=tuple(errors),
)
Loading