Skip to content

Remove unnecessary trace logs#221

Open
AnthonyRonning wants to merge 2 commits into
masterfrom
codex-opensecret-sensitive-log-redaction-opensecret
Open

Remove unnecessary trace logs#221
AnthonyRonning wants to merge 2 commits into
masterfrom
codex-opensecret-sensitive-log-redaction-opensecret

Conversation

@AnthonyRonning

@AnthonyRonning AnthonyRonning commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • remove trace output containing generated and active app-layer session keys
  • remove user and platform refresh-token values from logout trace output
  • retain platform password-reset metadata without including the reset code
  • add a source-level security invariant covering these secret-bearing log patterns

Context

The container image sets RUST_LOG=info, and the entrypoint preserves that configured value. This is preventive hardening; there is no evidence of production trace exposure.

Logout and refresh-token revocation behavior is unchanged.

Validation

  • cargo fmt --all -- --check
  • cargo test security_invariants::request_time_logs_do_not_include_session_keys_tokens_or_reset_codes -- --exact
  • cargo clippy --all-targets --all-features -- -D warnings
  • cargo test --all-features (214 passed, 16 database-backed tests ignored as expected)

Open in Devin Review

Summary by CodeRabbit

  • Security
    • Improved protection of sensitive authentication data by preventing session keys, refresh tokens, reset codes, and related secrets from appearing in trace logs.
    • Added automated checks to help ensure sensitive values remain excluded from request logging.

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Sensitive values were removed from password-reset, session, and logout trace logs. A source-scanning test now checks selected files for forbidden sensitive-data logging markers.

Changes

Sensitive logging safeguards

Layer / File(s) Summary
Remove sensitive values from traces
src/main.rs, src/web/attestation_routes.rs, src/web/login_routes.rs, src/web/platform/login_routes.rs, src/security_invariants.rs
Trace output no longer includes reset codes, session keys, or refresh tokens; a test scans selected source files for forbidden markers.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Poem

A bunny watched the trace logs glow,
Then hid the secrets down below.
No keys or tokens hop in sight,
Reset codes vanish from the light.
Safer logs now softly thump—
Hop, hop, hooray! 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: removing trace logs, especially sensitive ones.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex-opensecret-sensitive-log-redaction-opensecret

Comment @coderabbitai help to get the list of available commands.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

coderabbitai[bot]

This comment was marked as resolved.

@AnthonyRonning AnthonyRonning changed the title Stop logging application secrets Remove unnecessary trace logs Jul 14, 2026
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