Skip to content

fix(security): redact + status-map server errors; extract core::redact (H5) - #12

Merged
willamhou merged 1 commit into
masterfrom
sec/pr-0-core-redact
Jun 24, 2026
Merged

fix(security): redact + status-map server errors; extract core::redact (H5)#12
willamhou merged 1 commit into
masterfrom
sec/pr-0-core-redact

Conversation

@willamhou

Copy link
Copy Markdown
Owner

PR-D (folded with the prerequisite PR-0) of the security-hardening plan.

PR-0 — extract core::redact

Move the generic credential/secret redaction (Redactor, secret_values_of, value/key/scalar masking) out of the audit feature into an always-compiled core::redact. audit::redact now re-exports it and keeps only the audit-only AuditMeta. No behavior change; all audit tests still pass (re-export intact), and the redaction is now usable by non-audit code.

H5 — server error hygiene (/v1/exec)

The exec handler no longer returns raw upstream error text. error_response:

  • maps each AdapterError to a proper status — Auth→401, RateLimited→429 with a Retry-After header, NotFound/SiteNotFound→404, Unsupported→400, AuditUnavailable→503, Upstream→502, Other→500;
  • returns a structured { "error": { "kind", "message" } } body with the message redacted using this request's credential values (an upstream that echoes a credential in its error body can no longer leak it to the caller — the audit redaction boundary isn't bypassed on the error path);
  • the unknown-site path uses the same structured shape.

Success responses are unchanged (the agent's own data).

Tests

  • core::redact unit tests now run in the default build (no audit feature).
  • New handler tests: status mapping, credential redaction in error text, Retry-After header.
  • cargo test --workspace green; cargo test -p relais-core --features audit green (no test lost); cargo clippy clean.

Codex-reviewed; the flagged Retry-After and unknown-site-body items are included.

🤖 Generated with Claude Code

…t (H5)

PR-D (+ PR-0) of the security-hardening plan.

- PR-0: move the generic credential/secret redaction (Redactor, secret_values_of,
  value/key/scalar masking) out of the audit feature into an always-compiled
  `core::redact`; `audit::redact` re-exports it and keeps the audit-only AuditMeta.
- H5: /v1/exec no longer returns raw upstream error text. error_response maps each
  AdapterError to a proper HTTP status (Auth→401, RateLimited→429 +Retry-After,
  NotFound/SiteNotFound→404, Unsupported→400, AuditUnavailable→503, Upstream→502,
  Other→500) and returns a structured {error:{kind,message}} body with the message
  redacted using this request's credential values. The unknown-site path uses the
  same structured shape.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a347ccc2a2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

/// redacted body, and a `Retry-After` header for rate limits.
fn error_response(err: &AdapterError, credentials: &Option<Credentials>) -> Response {
let (status, kind) = classify(err);
let message = redacted_message(err, credentials);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Return a generic message for auth failures

When /v1/exec receives an AdapterError::Auth, this still copies err.to_string() into the public JSON body after only credential-value masking. In any adapter/auth backend that includes upstream auth diagnostics, account identifiers, or non-credential secrets in the Auth error text, the new 401 response still exposes those details; the H5 error-hygiene contract expects Auth failures to use a generic message rather than echoing adapter text. Special-case AdapterError::Auth(_) before constructing the body.

Useful? React with 👍 / 👎.

@willamhou
willamhou merged commit a27c34e into master Jun 24, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant