Wide events audit 2026-06-23#114
Conversation
Automated weekly logging audit. Artifact only: report at reports/wide-events/2026-06-23.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KQ4piFY3uGXQjoepoZaFiv
📝 WalkthroughWalkthroughAdds ChangesWide-events audit report
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ScreenshotsUpdated for
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 684535d340
ℹ️ 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".
| ## Summary | ||
|
|
||
| - Deterministic checks: pass | ||
| - Findings: 0 critical, 10 warning, 1 info |
There was a problem hiding this comment.
I checked the production warning surface with rg 'Log::warning' app resources; app/Actions/OpenProjectFromPathAction.php:36-39 logs project.registration.failed with the raw input path and $e->getMessage(), which matches the same privacy pattern this report flags for other actions but is absent from the findings. Because this human-gated audit undercounts the warnings and omits an actionable log path, readers can leave that privacy issue unfixed; please add the missing finding and update the count.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
reports/wide-events/2026-06-23.md (2)
116-122: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winResidual risks are reasonable and actionable.
The residual risks section identifies three concrete gaps: runtime context leakage (mitigation: owner-level Context::flush()), missing ownership attribution for some flows, and lack of a shared sanitization helper. The fourth risk (no cross-process log verification) is a deployment-stage concern. These are realistic and highlight areas for follow-up work.
One suggestion: line 119 mentions "several actions and services emit warnings but no owner has been identified" and gives
EnsureRfaGitExcludeActionas an example. Consider adding a follow-up task to systematically enumerate which actions/services lack an identified owner, so a future audit can track progress.🤖 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 `@reports/wide-events/2026-06-23.md` around lines 116 - 122, The "Residual Risks" section currently identifies missing ownership attribution as a gap but only provides one example (EnsureRfaGitExcludeAction). To make this actionable, add a follow-up task or recommendation after the residual risks that explicitly calls for a systematic enumeration of all actions and services that emit warnings but currently lack identified owners, so that future audits can track progress against this identified gap.
97-103: 🧹 Nitpick | 🔵 TrivialLogging configuration defaults diverge from doctrine; consider setting
LOG_CHANNEL=dailyandLOG_LEVEL=info.The configuration is confirmed:
- Default channel resolves to
stack(line 21), which defaults tosingle(line 56)- Both
singleanddailydrivers default toLOG_LEVEL=debug(lines 61, 67)This results in unbounded retention (
single) and a noisier level than the doctrine specifies. The suggested fix aligns the defaults: setLOG_CHANNELtodaily(14-day retention by default) andLOG_LEVELtoinfo.Remote channels (
slack,papertrail,syslog) remain unaffected by this change.🤖 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 `@reports/wide-events/2026-06-23.md` around lines 97 - 103, The logging configuration in the Laravel config has its defaults set to stack channel and debug log level, resulting in unbounded retention and excessive verbosity. To fix this, modify the default environment variable fallback for LOG_CHANNEL from stack to daily (which provides 14-day bounded retention), and change the default LOG_LEVEL from debug to info throughout the logging configuration drivers (both single and daily drivers). This aligns the actual defaults with the documented wide-events doctrine requirements while keeping remote channels like slack, papertrail, and syslog unaffected.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@reports/wide-events/2026-06-23.md`:
- Around line 49-95: Sanitize all warning logs to remove sensitive data across
the six affected files. Create a helper function that strips control characters,
caps length, and removes path content from error messages. In
LoadFileDiffAction, replace the raw stderr with a sanitized stderr_summary in
the git.diff.failed warning. In GetProjectStatusAction, replace repoPath with
project slug and stderr with the sanitized summary in the project.status.failed
warning. In GitMetadataService, add project_slug context at the action boundary
and replace all repo parameters with project slug and all error parameters with
error_class across all seven warning types (git.excludes_file.resolve_failed,
git.file_content.read_failed, git.ref.resolve_failed,
git.commit_parents.read_failed, git.root_commit.check_failed,
git.child_commit.read_failed, and git.commit_log.read_failed). In
EnsureRfaGitExcludeAction, replace repo with project slug and error with
error_class in both warning locations. In ResolveBranchBaseAction, replace repo
with project slug and error with error_class in the git.commit_range.list_failed
warning. In SyntaxHighlightService, replace the raw error message with
error_class and optional bounded summary in both tempest_highlighting_failed and
phiki_highlighting_failed warnings.
---
Nitpick comments:
In `@reports/wide-events/2026-06-23.md`:
- Around line 116-122: The "Residual Risks" section currently identifies missing
ownership attribution as a gap but only provides one example
(EnsureRfaGitExcludeAction). To make this actionable, add a follow-up task or
recommendation after the residual risks that explicitly calls for a systematic
enumeration of all actions and services that emit warnings but currently lack
identified owners, so that future audits can track progress against this
identified gap.
- Around line 97-103: The logging configuration in the Laravel config has its
defaults set to stack channel and debug log level, resulting in unbounded
retention and excessive verbosity. To fix this, modify the default environment
variable fallback for LOG_CHANNEL from stack to daily (which provides 14-day
bounded retention), and change the default LOG_LEVEL from debug to info
throughout the logging configuration drivers (both single and daily drivers).
This aligns the actual defaults with the documented wide-events doctrine
requirements while keeping remote channels like slack, papertrail, and syslog
unaffected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a8386ded-a952-422a-a053-672fd85e15b4
📒 Files selected for processing (1)
reports/wide-events/2026-06-23.md
| ### [WARNING] `git.diff.failed` warning passes raw stderr | ||
|
|
||
| - **File:** `app/Actions/LoadFileDiffAction.php:51-55` | ||
| - **Rule:** Mechanics > Warnings ("avoid raw file contents, raw stderr") and Agent check A9. | ||
| - **Evidence:** `Log::warning('git.diff.failed', ['reason' => 'diff_process_failed', 'path' => $path, 'stderr' => $e->stderr])`. | ||
| - **Impact:** Git stderr can echo absolute paths and file content fragments back to logs. The skill's worked example uses `stderr_summary` instead. | ||
| - **Suggested fix:** Replace `stderr` with a sanitized `stderr_summary` produced by a small helper that strips control characters and caps length. Drop any path content that the helper finds. | ||
|
|
||
| ### [WARNING] `project.status.failed` warning passes raw stderr and absolute repo path | ||
|
|
||
| - **File:** `app/Actions/GetProjectStatusAction.php:27-31` | ||
| - **Rule:** Mechanics > Warnings and Privacy. | ||
| - **Evidence:** Payload contains `'repoPath' => $repoPath` and `'stderr' => $e->stderr`. | ||
| - **Impact:** Absolute repo path and raw stderr land in the daily log on every status failure. Project slug is the standard preferred substitute and is reachable from `$repoPath`. | ||
| - **Suggested fix:** Replace `repoPath` with the project slug (or hashed path) and `stderr` with a sanitized summary. | ||
|
|
||
| ### [WARNING] Git metadata service warnings consistently emit raw `$e->getMessage()` and absolute repo path | ||
|
|
||
| - **File:** `app/Services/GitMetadataService.php:26-30,151-157,174-179,202-207,226-231,271-276,345-349` | ||
| - **Rule:** Mechanics > Warnings, Privacy, Agent check A9. | ||
| - **Evidence:** Every git failure warning uses the same shape: `'repo' => $repoPath` plus `'error' => $e->getMessage()`. Examples include `git.excludes_file.resolve_failed`, `git.file_content.read_failed`, `git.ref.resolve_failed`, `git.commit_parents.read_failed`, `git.root_commit.check_failed`, `git.child_commit.read_failed`, and `git.commit_log.read_failed`. | ||
| - **Impact:** Repeated absolute paths and raw git error strings pollute the local log. Many of these warnings fire during normal navigation so the noise is sustained. | ||
| - **Suggested fix:** Add the project context (slug) to `Context::add` at the owner boundary or pass a `project_slug` parameter into the service. Replace `error` with `error_class` plus an optional sanitized summary. | ||
|
|
||
| ### [WARNING] `EnsureRfaGitExcludeAction` warnings carry absolute repo path and raw exception message | ||
|
|
||
| - **File:** `app/Actions/EnsureRfaGitExcludeAction.php:52-57,76-81` | ||
| - **Rule:** Mechanics > Warnings and Privacy. | ||
| - **Evidence:** `'repo' => $repoPath` plus `'error' => $e->getMessage()` on both warning paths. | ||
| - **Impact:** Same shape as the git metadata service. Absolute paths are not justified here because a project slug is available from the calling action chain. | ||
| - **Suggested fix:** Prefer the project slug and replace `error` with `error_class`. | ||
|
|
||
| ### [WARNING] `ResolveBranchBaseAction` warning carries absolute repo path and raw exception message | ||
|
|
||
| - **File:** `app/Actions/ResolveBranchBaseAction.php:82-88` | ||
| - **Rule:** Mechanics > Warnings and Privacy. | ||
| - **Evidence:** `'repo' => $repoPath` plus `'error' => $e->getMessage()` on `git.commit_range.list_failed`. | ||
| - **Impact:** Same shape as the git metadata service warnings. | ||
| - **Suggested fix:** Replace with project slug and `error_class`. | ||
|
|
||
| ### [WARNING] `SyntaxHighlightService` warnings carry raw exception messages | ||
|
|
||
| - **File:** `app/Services/SyntaxHighlightService.php:247-251,555-558` | ||
| - **Rule:** Mechanics > Warnings and Privacy. | ||
| - **Evidence:** Both `syntax.highlighting.failed` warnings (`tempest_highlighting_failed` and `phiki_highlighting_failed`) include `'error' => $e->getMessage()`. | ||
| - **Impact:** Highlighter exceptions can carry the offending code snippet or path. The raw message is unbounded. | ||
| - **Suggested fix:** Replace `error` with `error_class` and an optional bounded summary. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that all files and line ranges in findings 5-10 exist and contain the cited violations.
echo "=== Finding 5: LoadFileDiffAction:51-55 ==="
sed -n '51,55p' app/Actions/LoadFileDiffAction.php
echo "=== Finding 6: GetProjectStatusAction:27-31 ==="
sed -n '27,31p' app/Actions/GetProjectStatusAction.php
echo "=== Finding 7: GitMetadataService (sample lines) ==="
sed -n '26,30p' app/Services/GitMetadataService.php
echo "=== Finding 8: EnsureRfaGitExcludeAction:52-57,76-81 ==="
sed -n '52,57p' app/Actions/EnsureRfaGitExcludeAction.php
sed -n '76,81p' app/Actions/EnsureRfaGitExcludeAction.php
echo "=== Finding 9: ResolveBranchBaseAction:82-88 ==="
sed -n '82,88p' app/Actions/ResolveBranchBaseAction.php
echo "=== Finding 10: SyntaxHighlightService:247-251,555-558 ==="
sed -n '247,251p' app/Services/SyntaxHighlightService.php
sed -n '555,558p' app/Services/SyntaxHighlightService.phpRepository: fgilio/rfa
Length of output: 2194
Sanitize sensitive data in warning logs across six files
Git operations, project status checks, and syntax highlighting log raw stderr, exception messages, and absolute repo paths. These can expose file paths and error details that don't belong in logs.
- LoadFileDiffAction:51-55 —
stderrlogs raw git diff output; use a sanitized summary instead. - GetProjectStatusAction:27-31 — Both
repoPathandstderrare logged; replace with project slug and error class. - GitMetadataService:26-30, 151-157, 174-179, 202-207, 226-231, 271-276, 345-349 — Seven git operation warnings all use
'repo' => $repoPathand'error' => $e->getMessage(); extract project slug viaContext::add()at the action boundary, replaceerrorwitherror_classplus optional bounded summary. - EnsureRfaGitExcludeAction:52-57, 76-81 — Same pattern: replace
repowith project slug,errorwitherror_class. - ResolveBranchBaseAction:82-88 — Replace
repowith project slug,errorwitherror_class. - SyntaxHighlightService:247-251, 555-558 — Both
tempest_highlighting_failedandphiki_highlighting_failedwarnings log rawerror; useerror_classand optional summary.
Add a small helper to produce sanitized error summaries (strip control characters, cap length, drop path content).
🤖 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 `@reports/wide-events/2026-06-23.md` around lines 49 - 95, Sanitize all warning
logs to remove sensitive data across the six affected files. Create a helper
function that strips control characters, caps length, and removes path content
from error messages. In LoadFileDiffAction, replace the raw stderr with a
sanitized stderr_summary in the git.diff.failed warning. In
GetProjectStatusAction, replace repoPath with project slug and stderr with the
sanitized summary in the project.status.failed warning. In GitMetadataService,
add project_slug context at the action boundary and replace all repo parameters
with project slug and all error parameters with error_class across all seven
warning types (git.excludes_file.resolve_failed, git.file_content.read_failed,
git.ref.resolve_failed, git.commit_parents.read_failed,
git.root_commit.check_failed, git.child_commit.read_failed, and
git.commit_log.read_failed). In EnsureRfaGitExcludeAction, replace repo with
project slug and error with error_class in both warning locations. In
ResolveBranchBaseAction, replace repo with project slug and error with
error_class in the git.commit_range.list_failed warning. In
SyntaxHighlightService, replace the raw error message with error_class and
optional bounded summary in both tempest_highlighting_failed and
phiki_highlighting_failed warnings.
2 participants
Automated weekly wide-events logging audit.
Artifact:
reports/wide-events/2026-06-23.mdThis PR is human-gated. It contains the report only.
Generated by Claude Code
Summary by CodeRabbit