fix(gateway): preserve OCI runtime failure context #264

Merged
jwilger merged 2 commits from fix/issue-148-oci-runtime-diagnostics into main 2026-05-19 05:43:35 -07:00
Owner

Why:

  • Operators need packaged OCI launch failures to distinguish spawn errors from runtime non-zero exits without exposing secret-bearing paths.
  • Closes #148.

What:

  • Redact known secret-bearing fragments from OCI setup diagnostics instead of replacing the entire message.
  • Preserve sanitized executor diagnostic context when packaged OCI runtime startup fails.
  • Add a focused regression test covering preserved context and redaction.

Validation:

  • rtk cargo test -p ar-gateway execute_packaged_oci_runtime_with_executor_preserves_sanitized_executor_context
  • just fmt
  • rtk cargo test -p ar-gateway
Why: - Operators need packaged OCI launch failures to distinguish spawn errors from runtime non-zero exits without exposing secret-bearing paths. - Closes #148. What: - Redact known secret-bearing fragments from OCI setup diagnostics instead of replacing the entire message. - Preserve sanitized executor diagnostic context when packaged OCI runtime startup fails. - Add a focused regression test covering preserved context and redaction. Validation: - `rtk cargo test -p ar-gateway execute_packaged_oci_runtime_with_executor_preserves_sanitized_executor_context` - `just fmt` - `rtk cargo test -p ar-gateway`
fix(gateway): preserve OCI runtime failure context
All checks were successful
CI / Classify changed paths (pull_request) Successful in 3s
CI / Format check (pull_request) Successful in 6s
CI / Clippy (pull_request) Successful in 39s
CI / Dependency policy (pull_request) Successful in 12s
CI / Test (pull_request) Successful in 51s
CI / opencode plugin tests (pull_request) Has been skipped
CI / Request auto_review semantic review (pull_request) Successful in 1s
auto_review auto_review: 1 warning
CI / Build (pull_request) Successful in 32s
CI / Build PR artifacts (no token) (pull_request) Has been skipped
2b6c8a93b7
Why:
- Operators need packaged OCI launch failures to distinguish spawn errors from runtime non-zero exits without exposing secret-bearing paths.
- See issue #148.

What:
- Sanitize OCI setup diagnostics by redacting secret-bearing fragments instead of replacing the whole message.
- Include sanitized executor diagnostics when packaged runtime startup fails.

Validation:
- rtk cargo test -p ar-gateway execute_packaged_oci_runtime_with_executor_preserves_sanitized_executor_context
- just fmt
- rtk cargo test -p ar-gateway
auto-review approved these changes 2026-05-18 23:11:18 -07:00
Dismissed
auto-review left a comment

The PR improves error handling by preserving context in OCI runtime failures while redacting sensitive information. The changes appear safe to merge, with added tests ensuring the new behavior works as intended.

Walkthrough

  • Redaction Logic:
    • The public_detail method now uses a new sanitize_oci_setup_detail function to redact sensitive information from error messages.
    • This change ensures that secret-bearing paths are replaced with [redacted] in diagnostic messages.
  • Error Handling:
    • The execute_packaged_oci_runtime_with_executor function now includes the sanitized diagnostic context in its error messages.
  • Testing:
    • A new test, execute_packaged_oci_runtime_with_executor_preserves_sanitized_executor_context, verifies that the error messages preserve necessary context while redacting sensitive information.

LLM usage and cost

The PR improves error handling by preserving context in OCI runtime failures while redacting sensitive information. The changes appear safe to merge, with added tests ensuring the new behavior works as intended. ## Walkthrough - **Redaction Logic**: - The `public_detail` method now uses a new `sanitize_oci_setup_detail` function to redact sensitive information from error messages. - This change ensures that secret-bearing paths are replaced with `[redacted]` in diagnostic messages. - **Error Handling**: - The `execute_packaged_oci_runtime_with_executor` function now includes the sanitized diagnostic context in its error messages. - **Testing**: - A new test, `execute_packaged_oci_runtime_with_executor_preserves_sanitized_executor_context`, verifies that the error messages preserve necessary context while redacting sensitive information. ## LLM usage and cost - Reasoning (gpt-4o) in=4177 out=295 cost=$0.025310 - Cheap (gpt-4o-mini) in=1501 out=99 cost=$0.000285 Estimated total USD: $0.025595 via https://api.openai.com and https://api.openai.com
@ -831,3 +831,2 @@
))
.map_err(|_diagnostic| {
OciSetupDiagnostic::new("packaged OCI runtime failed while starting the inner gateway")
.map_err(|diagnostic| {
Owner

🟡 Warning: Ensure that the diagnostic.public_detail() method is correctly sanitizing all sensitive information before being included in the error message.

🟡 **Warning:** Ensure that the `diagnostic.public_detail()` method is correctly sanitizing all sensitive information before being included in the error message.
jwilger marked this conversation as resolved
fix(gateway): redact OCI secret path suffixes
All checks were successful
CI / Classify changed paths (pull_request) Successful in 3s
CI / Clippy (pull_request) Successful in 40s
CI / opencode plugin tests (pull_request) Has been skipped
CI / Test (pull_request) Successful in 53s
CI / Format check (pull_request) Successful in 6s
CI / Dependency policy (pull_request) Successful in 12s
CI / Request auto_review semantic review (pull_request) Successful in 2s
CI / Build (pull_request) Successful in 33s
CI / Build PR artifacts (no token) (pull_request) Has been skipped
auto_review auto_review: 1 warning
e48ed1b699
Why:
- PR feedback identified that preserved OCI diagnostics must sanitize sensitive detail before surfacing executor context.
- Secret path names under /run/secrets can reveal credential purpose if only the prefix is redacted.

What:
- Redact the full /run/secrets/<name> path token while retaining safe operator context.
- Add focused coverage for preserving exit status context without leaking the secret path suffix.

Validation:
- rtk cargo test -p ar-gateway oci_setup_public_detail_fully_redacts_secret_path_suffix_and_keeps_safe_context
- just fmt
- rtk cargo test -p ar-gateway
jwilger dismissed auto-review's review 2026-05-18 23:20:41 -07:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

jwilger left a comment

Addressed sanitizer feedback.

Addressed sanitizer feedback.
Author
Owner

@auto-review Addressed. Reflection: I initially preserved context by replacing only known substrings, but did not consider that the suffix of a /run/secrets/<name> token can itself reveal sensitive credential purpose. Classification: one-off security review gap for this sanitizer, fixed with focused coverage rather than a new always-loaded guardrail. The follow-up commit now redacts the full /run/secrets/<name> token while preserving safe context such as exit status: 42. Validation: rtk cargo test -p ar-gateway oci_setup_public_detail_fully_redacts_secret_path_suffix_and_keeps_safe_context, just fmt, and rtk cargo test -p ar-gateway.

@auto-review Addressed. Reflection: I initially preserved context by replacing only known substrings, but did not consider that the suffix of a `/run/secrets/<name>` token can itself reveal sensitive credential purpose. Classification: one-off security review gap for this sanitizer, fixed with focused coverage rather than a new always-loaded guardrail. The follow-up commit now redacts the full `/run/secrets/<name>` token while preserving safe context such as `exit status: 42`. Validation: `rtk cargo test -p ar-gateway oci_setup_public_detail_fully_redacts_secret_path_suffix_and_keeps_safe_context`, `just fmt`, and `rtk cargo test -p ar-gateway`.
jwilger marked this conversation as resolved
auto-review left a comment

The PR improves the handling of OCI runtime failure diagnostics by redacting sensitive information while preserving useful context. The changes include a new function for sanitizing diagnostic details and additional test cases to ensure proper redaction and context preservation.

Walkthrough

  • Sanitization Functionality:
    • Introduced sanitize_oci_setup_detail to handle redaction of sensitive paths and keywords.
    • Added redact_run_secrets_path_suffix to specifically target and redact secret paths.
  • Error Handling:
    • Updated OciSetupDiagnostic::public_detail to use the new sanitization function.
    • Modified error handling in execute_packaged_oci_runtime_with_executor to include sanitized diagnostic details.
  • Testing:
    • Added test oci_setup_public_detail_fully_redacts_secret_path_suffix_and_keeps_safe_context to verify redaction and context preservation.
    • Added test execute_packaged_oci_runtime_with_executor_preserves_sanitized_executor_context to ensure executor context is preserved while redacting sensitive information.

LLM usage and cost

The PR improves the handling of OCI runtime failure diagnostics by redacting sensitive information while preserving useful context. The changes include a new function for sanitizing diagnostic details and additional test cases to ensure proper redaction and context preservation. ## Walkthrough - **Sanitization Functionality**: - Introduced `sanitize_oci_setup_detail` to handle redaction of sensitive paths and keywords. - Added `redact_run_secrets_path_suffix` to specifically target and redact secret paths. - **Error Handling**: - Updated `OciSetupDiagnostic::public_detail` to use the new sanitization function. - Modified error handling in `execute_packaged_oci_runtime_with_executor` to include sanitized diagnostic details. - **Testing**: - Added test `oci_setup_public_detail_fully_redacts_secret_path_suffix_and_keeps_safe_context` to verify redaction and context preservation. - Added test `execute_packaged_oci_runtime_with_executor_preserves_sanitized_executor_context` to ensure executor context is preserved while redacting sensitive information. ## LLM usage and cost - Reasoning (gpt-4o) in=5348 out=357 cost=$0.032095 - Cheap (gpt-4o-mini) in=1942 out=104 cost=$0.000354 Estimated total USD: $0.032449 via https://api.openai.com and https://api.openai.com
@ -829,8 +854,11 @@ fn execute_packaged_oci_runtime_with_executor(
inputs.runtime_path,
Owner

🟡 Warning: Ensure that diagnostic.public_detail() is correctly sanitizing all sensitive information before being included in the error message. This is crucial for maintaining security and privacy.

🟡 **Warning:** Ensure that `diagnostic.public_detail()` is correctly sanitizing all sensitive information before being included in the error message. This is crucial for maintaining security and privacy.
jwilger left a comment

Addressed current sanitizer warning.

Addressed current sanitizer warning.
Author
Owner

@auto-review Addressed. Reflection: the warning is a duplicate of the earlier sanitizer concern, and the first pass needed explicit coverage for full /run/secrets/<name> token redaction before reusing diagnostic.public_detail() in the wrapper error. Classification: one-off security review gap for this sanitizer; no new guardrail is warranted because the repository already requires threat-model/red-team attention for secret handling. The current code routes the executor diagnostic through diagnostic.public_detail(), and focused tests cover preserving safe runtime context while removing /run/secrets and secret-bearing markers. Validation already run on this branch: rtk cargo test -p ar-gateway execute_packaged_oci_runtime_with_executor_preserves_sanitized_executor_context, rtk cargo test -p ar-gateway oci_setup_public_detail_fully_redacts_secret_path_suffix_and_keeps_safe_context, just fmt, and rtk cargo test -p ar-gateway.

@auto-review Addressed. Reflection: the warning is a duplicate of the earlier sanitizer concern, and the first pass needed explicit coverage for full `/run/secrets/<name>` token redaction before reusing `diagnostic.public_detail()` in the wrapper error. Classification: one-off security review gap for this sanitizer; no new guardrail is warranted because the repository already requires threat-model/red-team attention for secret handling. The current code routes the executor diagnostic through `diagnostic.public_detail()`, and focused tests cover preserving safe runtime context while removing `/run/secrets` and secret-bearing markers. Validation already run on this branch: `rtk cargo test -p ar-gateway execute_packaged_oci_runtime_with_executor_preserves_sanitized_executor_context`, `rtk cargo test -p ar-gateway oci_setup_public_detail_fully_redacts_secret_path_suffix_and_keeps_safe_context`, `just fmt`, and `rtk cargo test -p ar-gateway`.
jwilger marked this conversation as resolved
jwilger deleted branch fix/issue-148-oci-runtime-diagnostics 2026-05-19 05:43:35 -07:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Slipstream/auto_review!264
No description provided.