Refs #576: Reject control chars in login CSV config#628
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughCSV environment variable parsing now detects control characters in individual items and adjusts how they are normalized before validation. When control characters are present, items are lowercased without trimming; otherwise items are trimmed and lowercased. A test validates that admin-login and labeler CSV entries with control characters are rejected by deploy readiness validation. ChangesCSV Control Character Detection
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
barnacleagent-svg
left a comment
There was a problem hiding this comment.
PR Review — #628
PR: #628
Author: @xingxi0614-cpu
Head SHA: d9189b3
Branch: codex/b576-config-login-controls
Files Changed
- (+7 lines): Added helper, applied to for admin/login CSV parsing
- (+21 lines): Added test covering tab and � control chars in login env vars
CI Status
Quality, readiness, docs, and image checks: completed/success
Code Review
The change is straightforward and well-scoped:
- Helper function correctly catches C0 (U+0000–U+001F) and C1 (U+007F–U+009F) control ranges
- Applied specifically to GitHub-login CSV env vars where control chars would be invisible but corrupt logins
- Test covers both and cases with proper deploy env setup
- No side effects on normal logins
Verdict
APPROVE — Clean, focused, well-tested. Ready for merge.
Refs #576
Summary
Reject raw control characters in deploy GitHub-login CSV entries before trimming them into clean admin or accepted-labeler logins.
This keeps values such as these from being accepted as
aliceduring deploy readiness checks:MERGEWORK_ADMIN_LOGINS=<tab>aliceMERGEWORK_GITHUB_ACCEPTED_LABELERS=alice\x85Normal clean entries and ordinary comma-separated login behavior remain unchanged.
Distinctness
This is limited to the admin/accepted-labeler GitHub-login CSV parsing path. It does not overlap the existing #576 slices for deploy secret/database/base URL C1 controls, webhook identities, wallet material, account/wallet filters, bounty filters, MCP arguments, JSON integers, MRWK amounts, or attempt source URLs.
Validation
tests/test_deploy_readiness.py: 28 passedmypy app/config.py: successgit diff --check: cleanorigin/mainand PR Refs #576: Reject C1 controls in deploy settings #623Safety
No private data, secrets, admin token values, wallet material, production mutation, price/liquidity/exchange/bridge/off-ramp claims, or fabricated payout claims are included.
Summary by CodeRabbit