Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Правило "Запуск внешних приложений" - ExternalAppStarting #3115

Merged
merged 8 commits into from
Dec 26, 2023

Conversation

artbear
Copy link
Contributor

@artbear artbear commented Jul 24, 2023

Описание

  • Реализовано новое правило "Запуск внешних приложений" - ExternalAppStarting

  • для исключения ФП нужно добавить флаг для срабатывания на ПерейтиПоНавигационнойСсылке или ФайловаяСистемаКлиент.ОткрытьНавигационнуюСсылку

  • Скорее всего, нужен еще параметр правила для глобальных методов и методов объектов
    чтобы добавлять новые методы, например, из типовых конфигураций или БСП

  • Добавить ЗапуститьСистему

  • Отдельная проверка на COM-объекты не нужна
    например, на все (лучше) или только "Wscript.Shell" и "Shell.Application" (хуже)

Связанные задачи

Closes #3114

Чеклист

Общие

  • Ветка PR обновлена из develop
  • Отладочные, закомментированные и прочие, не имеющие смысла участки кода удалены
  • Изменения покрыты тестами
  • Обязательные действия перед коммитом выполнены (запускал команду gradlew precommit)

Для диагностик

  • Описание диагностики заполнено для обоих языков (присутствуют файлы для обоих языков, для русского заполнено все подробно, перевод на английский можно опустить)

Дополнительно

Summary by CodeRabbit

  • New Features

    • Introduced a new security diagnostic for detecting unsafe methods of starting external applications.
  • Documentation

    • Added documentation for the new diagnostic feature, including description, usage examples, and references.
  • Localization

    • Provided English and Russian translations for diagnostic messages and user patterns.
  • Tests

    • Implemented test cases to ensure the reliability of the new diagnostic feature.

@ghost
Copy link

ghost commented Jul 24, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@artbear
Copy link
Contributor Author

artbear commented Jul 25, 2023

@nixel2007 @theshadowco @otymko Правило готово, проверяйте.

@sonarcloud
Copy link

sonarcloud bot commented Jul 25, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@artbear artbear changed the title Правило "Запуск внешних приложений" - ExternalAppStarting Правило "Запуск внешних приложений" - ExternalAppStarting - ГОТОВО Aug 24, 2023
@artbear artbear changed the title Правило "Запуск внешних приложений" - ExternalAppStarting - ГОТОВО Правило "Запуск внешних приложений" - ExternalAppStarting Oct 15, 2023
@artbear artbear marked this pull request as draft October 15, 2023 16:49
переделаны тесты
выполнен precommit
@artbear artbear marked this pull request as ready for review December 23, 2023 06:58
Copy link

coderabbitai bot commented Dec 23, 2023

Walkthrough

The recent updates introduce a diagnostic feature to identify and manage security risks associated with the execution of external applications from 1C code. This enhancement aims to extend security measures, covering various methods of launching external programs and providing a mechanism to avoid false positives, with a focus on maintaining the security and integrity of the codebase.

Changes

File Path Change Summary
docs/diagnostics/.../ExternalAppStarting.md Added documentation for diagnostics of external application launching.
src/main/java/.../ExternalAppStartingDiagnostic.java New diagnostic for detecting unsafe external application starts in BSL code.
src/main/resources/.../ExternalAppStartingDiagnostic_en.properties
src/main/resources/.../ExternalAppStartingDiagnostic_ru.properties
Introduced diagnostic messages and localization for launching external applications.
src/test/java/.../ExternalAppStartingDiagnosticTest.java Implemented tests for the new ExternalAppStartingDiagnostic class.
src/test/resources/diagnostics/ExternalAppStartingDiagnostic.bsl Added BSL procedures and functions for testing external application launch.

Assessment against linked issues

Objective Addressed Explanation
Implement security controls for execution of external applications from 1C code (#3114)
Extend security rule to cover various methods of launching external programs (#3114)
Add flag to handle potential false positives for specific methods (#3114)
Evaluate additional parameter for global and object methods (#3114) It's unclear if a new parameter for global methods has been added without looking at the code details.
Exclude separate check for COM objects in Unix-compatible code (#3114)

Poem

In the realm of code where rabbits dwell,
A new sentinel rises, so all is well. 🛡️
External apps at bay, it keeps,
Guarding the code while the world sleeps. 🌙✨

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ?


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • You can directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
  • You can tag CodeRabbit on specific lines of code or entire files in the PR by tagging @coderabbitai in a comment. Examples:
  • You can tag @coderabbitai in a PR comment and ask questions about the PR and the codebase. Use quoted replies to pass the context for follow-up questions. Examples:
    • @coderabbitai render interesting statistics about this repository as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai generate unit tests for the src/utils.ts file.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 5796c10 and f378e30.
Files ignored due to filter (1)
  • src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/parameters-schema.json
Files selected for processing (7)
  • docs/diagnostics/ExternalAppStarting.md (1 hunks)
  • docs/en/diagnostics/ExternalAppStarting.md (1 hunks)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ExternalAppStartingDiagnostic.java (1 hunks)
  • src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/ExternalAppStartingDiagnostic_en.properties (1 hunks)
  • src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/ExternalAppStartingDiagnostic_ru.properties (1 hunks)
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ExternalAppStartingDiagnosticTest.java (1 hunks)
  • src/test/resources/diagnostics/ExternalAppStartingDiagnostic.bsl (1 hunks)
Files skipped from review due to trivial changes (2)
  • docs/en/diagnostics/ExternalAppStarting.md
  • src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/ExternalAppStartingDiagnostic_ru.properties
Additional comments: 5
src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/ExternalAppStartingDiagnostic_en.properties (1)
  • 1-4: The English localization for the diagnostic messages is correctly formatted and consistent with the diagnostic's purpose.
src/test/resources/diagnostics/ExternalAppStartingDiagnostic.bsl (1)
  • 2-58: The BSL script file contains comprehensive test cases for the diagnostic, covering a variety of methods that should be flagged.
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ExternalAppStartingDiagnostic.java (1)
  • 35-79: The Java class implementing the diagnostic correctly declares parameters and the configure method appends the pattern for navigation links based on the checkGotoUrl parameter.
docs/diagnostics/ExternalAppStarting.md (1)
  • 1-78: The Markdown documentation for the diagnostic is clear and provides useful examples and sources.
src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ExternalAppStartingDiagnosticTest.java (1)
  • 37-131: The Java test class for the diagnostic includes tests that cover its behavior with different configurations.

@theshadowco theshadowco merged commit 29390f1 into 1c-syntax:develop Dec 26, 2023
19 checks passed
@artbear artbear deleted the ExternalAppStarting branch December 27, 2023 04:47
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.

[NEW] Запуск внешних приложений
2 participants