feat(eol-normalizer): disclose line-ending rewrites on the user channel - #2458
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Claude finished @kyle-sexton's task in 1m 20s —— View job Security Review
Scope: No security issues found. Details on what I checked:
|
![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: ca98197e21
ℹ️ 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".
3f54c3b to
5a5a997
Compare
|
Claude finished @kyle-sexton's task in 1m 0s —— View job Security Review
Scope: No security issues found. Details on what I checked:
Note: Codex flagged that the disclosure can fire even when the file's bytes didn't actually change (idempotent |
|
Claude finished @kyle-sexton's task in 1m 49s —— View job Code Review
Scope: 🔴 Finding: disclosure fires on no-op writes, contradicting the PR's own "stay silent on skip/no-op paths" contractPosted inline on Other areas checked, no issues found
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Claude has reviewed this PR 5 times. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
5a5a997 to
e596a81
Compare
|
Claude finished @kyle-sexton's task in 59s —— View job Security Review
Scope: No security issues found. What I checked:
Note: there's a known correctness issue (already reported by both Codex and a prior Claude review on this same line) where the disclosure can fire on idempotent/no-op writes where bytes didn't actually change. That's a functional accuracy bug against the PR's stated "stay silent on no-op paths" intent, not a security issue (no injection, no unauthorized disclosure, no trust-boundary effect), so it's out of scope for this security-focused pass. |
845dc3d to
321c6d0
Compare
|
Claude finished @kyle-sexton's task in 3m 7s —— View job Security Review
Scope: 🟡 Finding: idempotency-check temp copy is written to a shared
|
|
Claude finished @kyle-sexton's task in 3m 44s —— View job Code Review
Scope: Latest commit ( 🔴 Test regression: fixing that bug appears to have broken a pre-existing testPosted inline on 🟡 Perf/architecture: the new snapshot runs unconditionally on every write, not just eligible onesPosted inline on Other areas checked, no issues found
|
321c6d0 to
a216239
Compare
|
Claude finished @cursor[bot]'s task in 1m 58s —— View job Security Review
Scope: 🟡 IMPORTANT — idempotency-check temp copy still lands in shared
|
|
Last security-reviewed head: |
|
Claude finished @cursor[bot]'s task in 2m 13s —— View job Code Review
Scope: I confirmed the current HEAD's 🔴 Test regression (posted inline on
|
d87b45a to
8d0f005
Compare
|
Claude finished @kyle-sexton's task in 1m 37s —— View job Security Review
Scope: 🟡 IMPORTANT — idempotency-check temp copy still lands in shared
|
|
Claude finished @kyle-sexton's task in 1m 23s —— View job Security Review
Skipping — no new security-relevant content since the last full review. I diffed For the record, the one still-open item from the prior rounds remains unchanged:
No new findings this round. |
|
Claude finished @kyle-sexton's task in 2m 30s —— View job Code Review
Scope: HEAD 🔴 Test regression —
|
|
Claude finished @cursor[bot]'s task in 1m 12s —— View job Security Review
Skipping — no new security-relevant content since the last full review. I diffed the last security-reviewed head ( For the record, the one still-open item from prior rounds remains unchanged at
No new findings this round. |
|
Claude finished @cursor[bot]'s task in 1m 54s —— View job Code Review (
|
…el (#1256) Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
b3a406b to
0f9ed1c
Compare
Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
|
Claude finished @cursor[bot]'s task in 1m 17s —— View job Security Review
Skipping — no new security-relevant content since the last full review (skip gate condition 3). I diffed the last security-reviewed head ( - **Content-mutation disclosure on the user channel (#1596).** When the hook normalizes line
- endings to LF or CRLF, it now emits a `systemMessage` naming the file and the target ending.
- Skip and no-op paths stay silent.
+ **Line-ending rewrites are disclosed on the user channel** when the hook normalizes
+ CRLF/LF mismatches, so agents see the mutation instead of silently rewriting (#1256).
For the record, the one still-open item from prior rounds remains unchanged at
No new findings this round. |
tel2.sh is normalized in the telemetry stub-sink case; re-running the hook on it is a no-op and correctly omits disclosure. Use tel2b.sh instead. Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
|
Claude finished @cursor[bot]'s task in 56s —— View job Security Review
Skipping — no new security-relevant content since the last full review (skip gate condition 2/3). I diffed the last security-reviewed head ( plugins/eol-normalizer/hooks/eol-normalizer.test.sh | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)- OUT_LF=$(run_hook_env "$REPO/tel2.sh" CLAUDE_PLUGIN_OPTION_EOL_NORMALIZER_ENABLED=true)
+ # Fresh file: tel2.sh was already normalized in the telemetry stub-sink case above.
+ printf 'echo r\r\n' >"$REPO/tel2b.sh"
+ OUT_LF=$(run_hook_env "$REPO/tel2b.sh" CLAUDE_PLUGIN_OPTION_EOL_NORMALIZER_ENABLED=true)This is a test-only fixture fix (seeds a fresh CRLF file, For the record, the one still-open item from prior rounds remains unchanged at
No new findings this round. |
2 participants
No linked issue
Summary
systemMessagewhen the hook normalizes line endings to LF or CRLFPartially addresses #1596 (eol-normalizer only; sibling autofix hooks remain tracked there).
Test plan
bash plugins/eol-normalizer/hooks/eol-normalizer.test.sh(36 passed)Related
Refs #1596