fix(cli): guard against accidental Spring Boot debug mode#4244
Merged
Conversation
Passing --debug/--trace or setting the DEBUG/TRACE environment variables (Spring binds them to the debug/trace properties via relaxed binding) turns on verbose core-logger logging plus the auto-configuration report. Over LSP stdio that stray output can corrupt the protocol channel, and DEBUG is a common env var that is easy to set by accident. Spring Boot does not let you rename or disable the debug/trace keys, so guard them before the context boots: honor the request only when the explicit --enable-debug-i-know-what-i-am-doing opt-in is given on the command line. Otherwise strip the flags from the args and force debug/trace to false via system properties (higher precedence than env vars), warning on stderr. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01JEuMeqWyv8FsPMfEX5pCR7
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughMainApplication now guards Spring Boot debug and trace handling behind an explicit opt-in flag. It filters related CLI args, forces debug and trace properties off when not opted in, and updates tests plus the standard-stream rule to cover the new behavior. ChangesSpring Debug/Trace Guard
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
The debug-mode guard in MainApplication runs in main() before Spring configures logging and writes its warning to stderr (stdout is the LSP protocol channel), so it must be exempted from no_classes_should_access_ standard_streams — same rationale as the ParentProcessWatcher fallback. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01JEuMeqWyv8FsPMfEX5pCR7
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/test/java/com/github/_1c_syntax/bsl/languageserver/MainApplicationModeTest.java (1)
116-130: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider covering the environment/property-driven debug path.
The four tests exercise the CLI-flag paths well, but the
debugRequestedFromEnvironment()branch (no--debug/--traceflag, yetdebug/tracerequested via property) is untested. Since env vars are hard to set in-test, a property-based case would close the gap:💚 Suggested additional test
`@Test` void debugFromSystemPropertyForcedOff() { System.clearProperty("debug"); System.clearProperty("trace"); try { System.setProperty("debug", "true"); var result = MainApplication.guardSpringDebugMode(new String[]{"analyze"}); assertThat(result).containsExactly("analyze"); assertThat(System.getProperty("debug")).isEqualTo("false"); } finally { System.clearProperty("debug"); System.clearProperty("trace"); } }🤖 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 `@src/test/java/com/github/_1c_syntax/bsl/languageserver/MainApplicationModeTest.java` around lines 116 - 130, Add a test that covers the property-driven debug branch in MainApplication.guardSpringDebugMode, since only CLI-flag paths are currently exercised. Create a case where no --debug/--trace flag is passed but the debug state is requested via system properties (for example by setting debug before calling the method), then assert the returned args stay unchanged and that the debug property is forced off as expected. Keep the existing cleanup around System.clearProperty for debug and trace.
🤖 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.
Nitpick comments:
In
`@src/test/java/com/github/_1c_syntax/bsl/languageserver/MainApplicationModeTest.java`:
- Around line 116-130: Add a test that covers the property-driven debug branch
in MainApplication.guardSpringDebugMode, since only CLI-flag paths are currently
exercised. Create a case where no --debug/--trace flag is passed but the debug
state is requested via system properties (for example by setting debug before
calling the method), then assert the returned args stay unchanged and that the
debug property is forced off as expected. Keep the existing cleanup around
System.clearProperty for debug and trace.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6bb21a13-2609-4e76-99d6-e4a63eace247
📒 Files selected for processing (2)
src/main/java/com/github/_1c_syntax/bsl/languageserver/MainApplication.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/MainApplicationModeTest.java
Add a case where debug is requested via the debug system property (no --debug/--trace flag), exercising debugRequestedFromEnvironment(): args stay unchanged and debug is forced off. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01JEuMeqWyv8FsPMfEX5pCR7
Contributor
Test Results 3 588 files 3 588 suites 1h 48m 50s ⏱️ Results for commit 3e03772. ♻️ This comment has been updated with latest results. |
nixel2007
commented
Jul 8, 2026
Per review: the inline comment above the --debug/--trace patterns was redundant. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01JEuMeqWyv8FsPMfEX5pCR7
nixel2007
commented
Jul 8, 2026
nixel2007
commented
Jul 8, 2026
nixel2007
commented
Jul 8, 2026
nixel2007
commented
Jul 8, 2026
Per review: - on explicit opt-in, return args unchanged instead of filtering them (the opt-in flag and --debug/--trace are whitelisted for picocli); - match the opt-in option and --debug/--trace with single regexes, dropping the duplicated equalsIgnoreCase / regionMatches helpers; - when not opted in, force debug/trace properties off unconditionally, which removes the separate environment-detection and isEnabled helpers. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01JEuMeqWyv8FsPMfEX5pCR7
|
Member
Author
|
/buildJar |
Contributor
|
✅ Собраны JAR-файлы для этого PR по команде Артефакт: 8172457277 Файлы внутри:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Описание
Защита от случайного включения отладочного режима Spring Boot.
Флаги
--debug/--trace, а также переменные средыDEBUG/TRACE(Spring привязывает их к свойствамdebug/traceпо relaxed binding) включают объёмный лог core-логгеров и отчёт об автоконфигурации. Для LSP по stdio это особенно опасно: посторонний вывод способен замусорить канал протокола. При этомDEBUG— распространённое имя переменной среды, её легко выставить в окружении случайно.Переименовать или отключить ключи
debug/traceна уровне Spring Boot нельзя (они захардкожены вLoggingApplicationListener), поэтому добавлен внешний гвард вMainApplication.main(), срабатывающий до старта контекста:--debug/--traceубираются из аргументов, а свойстваdebug/traceпринудительно ставятся вfalseчерез системные свойства (они приоритетнее переменных среды — так гасится случайныйDEBUG/TRACE). Наstderrпечатается предупреждение с подсказкой, как включить осознанно.--enable-debug-i-know-what-i-am-doing=true(учитывается только из командной строки) отладка пропускается как есть; сама опция удаляется из аргументов, чтобы её не увидели picocli/Spring.allowedAdditionalArgsдобавлен--trace(и форма=value), чтобы при осознанном включении picocli не считал флаг неизвестной опцией.Связанные задачи
Closes
Чеклист
Общие
gradlew precommit)Для диагностик
Дополнительно
Новые тесты в
MainApplicationModeTest: снятие флагов без опции с форсомdebug/trace=false, сохранение флагов при осознанном включении,=falseне включает отладку, no-op без запроса на отладку.Замечание по проверке: в окружении не удалось прогнать
./gradlew— дистрибутив Gradle 9.6.1 блокируется egress-политикой прокси (403 наservices.gradle.org), а системный Gradle 8.14.3 не парсит текущийbuild.gradle.kts. Фактический прогон тестов остаётся за CI.🤖 Generated with Claude Code
https://claude.ai/code/session_01JEuMeqWyv8FsPMfEX5pCR7
Generated by Claude Code
Summary by CodeRabbit
--debug/--trace(including=...forms) when not allowed.