fix: removed unsafe `shell` when executing process #2067

kittaakos · 2023-05-11T16:03:55Z

Motivation

To eliminate the unsafe shell: true options when spawning processes. No IDE2 functionality changes are expected.

Change description

removed unnecessary chai-string and the corresponding type declarations,
cleaned up getExecPath function and removed another dependency: which, and
added tests for each executable provided by IDE2.

Other information

Steps to verify:

please put IDE2 on a path with spaces❗,
CLI is functional, LS works, and you can format the sketch,
you can flash with the firmware uploader

Ref: PNX-3671

Reviewer checklist

PR addresses a single concern.
The PR has no duplicates (please search among the Pull Requests before creating one)
PR title and description are properly filled.
Docs have been added / updated (for bug fixes / features)

rhpco · 2023-05-12T07:14:45Z

arduino-ide-extension/src/node/exec-util.ts

@@ -55,7 +21,7 @@ export function spawnCommand(
  stdIn?: string
 ): Promise<string> {
  return new Promise<string>((resolve, reject) => {
-    const cp = spawn(command, args, { windowsHide: true, shell: true });
+    const cp = spawn(command, args, { windowsHide: true });


per1234

Auto Format operation fails

The invocation of clang-format when using the "Auto Format" feature fails with the error: "Invalid value for -style"

To reproduce

Create a sketch that would be modified by an "Auto Format" operation:
```
void setup (){
//foo
}
void loop() {}
```
Select Tools > Auto Format from the Arduino IDE menus.
🐛 The sketch is not formatted.
Check the logs.

🐛 The logs show the clang-format command errored:

2023-05-15T06:56:48.633Z root ERROR Request format failed with error: Error executing E:\incoming\review\2067\1-e203c61\resources\app\node_modules\arduino-ide-extension\build\clang-format.exe -style="{\"AccessModifierOffset\":-2,\"AlignAfterOpenBracket\":\"Align\",\"AlignArrayOfStructures\":\"None\",\"AlignConsecutiveAssignments\":\"None\",\"AlignConsecutiveBitFields\":\"None\",\"AlignConsecutiveDeclarations\":\"None\",\"AlignConsecutiveMacros\":\"None\",\"AlignEscapedNewlines\":\"DontAlign\",\"AlignOperands\":\"Align\",\"AlignTrailingComments\":true,\"AllowAllArgumentsOnNextLine\":true,\"AllowAllConstructorInitializersOnNextLine\":true,\"AllowAllParametersOfDeclarationOnNextLine\":true,\"AllowShortBlocksOnASingleLine\":\"Always\",\"AllowShortCaseLabelsOnASingleLine\":true,\"AllowShortEnumsOnASingleLine\":true,\"AllowShortFunctionsOnASingleLine\":\"Empty\",\"AllowShortIfStatementsOnASingleLine\":\"AllIfsAndElse\",\"AllowShortLambdasOnASingleLine\":\"Empty\",\"AllowShortLoopsOnASingleLine\":true,\"AlwaysBreakAfterDefinitionReturnType\":\"None\",\"AlwaysBreakAfterReturnType\":\"None\",\"AlwaysBreakBeforeMultilineStrings\":false,\"AlwaysBreakTemplateDeclarations\":\"No\",\"AttributeMacros\":[\"__capability\"],\"BasedOnStyle\":\"LLVM\",\"BinPackArguments\":true,\"BinPackParameters\":true,\"BitFieldColonSpacing\":\"Both\",\"BraceWrapping\":{\"AfterCaseLabel\":false,\"AfterClass\":false,\"AfterControlStatement\":\"Never\",\"AfterEnum\":false,\"AfterFunction\":false,\"AfterNamespace\":false,\"AfterObjCDeclaration\":false,\"AfterStruct\":false,\"AfterUnion\":false,\"AfterExternBlock\":false,\"BeforeCatch\":false,\"BeforeElse\":false,\"BeforeLambdaBody\":false,\"BeforeWhile\":false,\"IndentBraces\":false,\"SplitEmptyFunction\":true,\"SplitEmptyRecord\":true,\"SplitEmptyNamespace\":true},\"BreakAfterJavaFieldAnnotations\":false,\"BreakBeforeBinaryOperators\":\"NonAssignment\",\"BreakBeforeBraces\":\"Attach\",\"BreakBeforeConceptDeclarations\":false,\"BreakBeforeInheritanceComma\":false,\"BreakBeforeTernaryOperators\":true,\"BreakConstructorInitializers\":\"BeforeColon\",\"BreakConstructorInitializersBeforeComma\":false,\"BreakInheritanceList\":\"BeforeColon\",\"BreakStringLiterals\":false,\"ColumnLimit\":0,\"CommentPragmas\":\"\",\"CompactNamespaces\":false,\"ConstructorInitializerAllOnOneLineOrOnePerLine\":false,\"ConstructorInitializerIndentWidth\":2,\"ContinuationIndentWidth\":2,\"Cpp11BracedListStyle\":false,\"DeriveLineEnding\":true,\"DerivePointerAlignment\":true,\"DisableFormat\":false,\"EmptyLineAfterAccessModifier\":\"Leave\",\"EmptyLineBeforeAccessModifier\":\"Leave\",\"ExperimentalAutoDetectBinPacking\":false,\"FixNamespaceComments\":false,\"ForEachMacros\":[\"foreach\",\"Q_FOREACH\",\"BOOST_FOREACH\"],\"IfMacros\":[\"KJ_IF_MAYBE\"],\"IncludeBlocks\":\"Preserve\",\"IncludeCategories\":[{\"Regex\":\"^^\\\"(llvm|llvm-c|clang|clang-c)/\",\"Priority\":2,\"SortPriority\":0,\"CaseSensitive\":false},{\"Regex\":\"^(<|\\\"(gtest^|gmock^|isl^|json)/)\",\"Priority\":3,\"SortPriority\":0,\"CaseSensitive\":false},{\"Regex\":\".*\",\"Priority\":1,\"SortPriority\":0,\"CaseSensitive\":false}],\"IncludeIsMainRegex\":\"\",\"IncludeIsMainSourceRegex\":\"\",\"IndentAccessModifiers\":false,\"IndentCaseBlocks\":true,\"IndentCaseLabels\":true,\"IndentExternBlock\":\"Indent\",\"IndentGotoLabels\":false,\"IndentPPDirectives\":\"None\",\"IndentRequires\":true,\"IndentWidth\":2,\"IndentWrappedFunctionNames\":false,\"InsertTrailingCommas\":\"None\",\"JavaScriptQuotes\":\"Leave\",\"JavaScriptWrapImports\":true,\"KeepEmptyLinesAtTheStartOfBlocks\":true,\"LambdaBodyIndentation\":\"Signature\",\"Language\":\"Cpp\",\"MacroBlockBegin\":\"\",\"MacroBlockEnd\":\"\",\"MaxEmptyLinesToKeep\":100000,\"NamespaceIndentation\":\"None\",\"ObjCBinPackProtocolList\":\"Auto\",\"ObjCBlockIndentWidth\":2,\"ObjCBreakBeforeNestedBlockParam\":true,\"ObjCSpaceAfterProperty\":false,\"ObjCSpaceBeforeProtocolList\":true,\"PPIndentWidth\":-1,\"PackConstructorInitializers\":\"BinPack\",\"PenaltyBreakAssignment\":1,\"PenaltyBreakBeforeFirstCallParameter\":1,\"PenaltyBreakComment\":1,\"PenaltyBreakFirstLessLess\":1,\"PenaltyBreakOpenParenthesis\":1,\"PenaltyBreakString\":1,\"PenaltyBreakTemplateDeclaration\":1,\"PenaltyExcessCharacter\":1,\"PenaltyIndentedWhitespace\":1,\"PenaltyReturnTypeOnItsOwnLine\":1,\"PointerAlignment\":\"Right\",\"QualifierAlignment\":\"Leave\",\"ReferenceAlignment\":\"Pointer\",\"ReflowComments\":false,\"RemoveBracesLLVM\":false,\"SeparateDefinitionBlocks\":\"Leave\",\"ShortNamespaceLines\":0,\"SortIncludes\":\"Never\",\"SortJavaStaticImport\":\"Before\",\"SortUsingDeclarations\":false,\"SpaceAfterCStyleCast\":false,\"SpaceAfterLogicalNot\":false,\"SpaceAfterTemplateKeyword\":false,\"SpaceAroundPointerQualifiers\":\"Default\",\"SpaceBeforeAssignmentOperators\":true,\"SpaceBeforeCaseColon\":false,\"SpaceBeforeCpp11BracedList\":false,\"SpaceBeforeCtorInitializerColon\":true,\"SpaceBeforeInheritanceColon\":true,\"SpaceBeforeParens\":\"ControlStatements\",\"SpaceBeforeParensOptions\":{\"AfterControlStatements\":true,\"AfterForeachMacros\":true,\"AfterFunctionDefinitionName\":false,\"AfterFunctionDeclarationName\":false,\"AfterIfMacros\":true,\"AfterOverloadedOperator\":false,\"BeforeNonEmptyParentheses\":false},\"SpaceBeforeRangeBasedForLoopColon\":true,\"SpaceBeforeSquareBrackets\":false,\"SpaceInEmptyBlock\":false,\"SpaceInEmptyParentheses\":false,\"SpacesBeforeTrailingComments\":2,\"SpacesInAngles\":\"Leave\",\"SpacesInCStyleCastParentheses\":false,\"SpacesInConditionalStatement\":false,\"SpacesInContainerLiterals\":false,\"SpacesInLineCommentPrefix\":{\"Minimum\":0,\"Maximum\":-1},\"SpacesInParentheses\":false,\"SpacesInSquareBrackets\":false,\"Standard\":\"Auto\",\"StatementAttributeLikeMacros\":[\"Q_EMIT\"],\"StatementMacros\":[\"Q_UNUSED\",\"QT_REQUIRE_VERSION\"],\"TabWidth\":2,\"UseCRLF\":false,\"UseTab\":\"Never\",\"WhitespaceSensitiveMacros\":[\"STRINGIZE\",\"PP_STRINGIZE\",\"BOOST_PP_STRINGIZE\",\"NS_SWIFT_NAME\",\"CF_SWIFT_NAME\"]}": Invalid value for -style

Expected behavior

No error from the generated clang-format command

Arduino IDE version

e203c61 (tester build for d69174a)

Operating system

Windows 11

Additional context

The fault does not occur when using the latest build from the main branch (117b2a4).

The fault occurs even with the IDE installed under a path that does not contain spaces.

kittaakos · 2023-05-16T14:50:11Z

Thank you, Per. The latest version of this PR should contain the fix. Just so you know, I removed the unique styles handling on Windows. (See #1285.) I covered the clang-format functionality with tests. They pass on both Windows and POSIX. If you can reproduce #1285, please let me know. No formatter-related changes are expected (compared with main) after my latest modifications.

kittaakos · 2023-05-24T16:16:51Z

Please review. Thanks!

arduino-ide-extension/src/node/clang-formatter.ts

davegarthsimpson

Code reviewed, no issues identified.

I've run the related tests locally on my intel mac and they pass.

I've also tested the IDE2 locally on the same machine and confirm the verify steps above are working, including an upload, and firmware update of the MKR WiFi 1010.

Ref: PNX-3671 Co-authored-by: per1234 <accounts@perglass.com> Co-authored-by: Akos Kitta <a.kitta@arduino.cc> Signed-off-by: Akos Kitta <a.kitta@arduino.cc>

per1234

Thanks Akos! I'm very glad that you were able to remove that hacky clang-format -style flag argument quoting code for the Windows command interpreter as part of this work.

Use a `string` array of command flags instead of the concatenated final `string` command. Ref: #2067 Closes #2179 Signed-off-by: Akos Kitta <a.kitta@arduino.cc>

Closes #2112 Ref: #2067 Signed-off-by: Akos Kitta <a.kitta@arduino.cc>

Use a `string` array of command flags instead of the concatenated final `string` command. Ref: #2067 Closes #2179 Signed-off-by: Akos Kitta <a.kitta@arduino.cc>

Closes #2112 Ref: #2067 Signed-off-by: Akos Kitta <a.kitta@arduino.cc>

kittaakos added topic: code Related to content of the project itself topic: security Related to the protection of user data type: imperfection Perceived defect in any part of project labels May 11, 2023

kittaakos requested review from rhpco, AlbyIanna and per1234 May 11, 2023 16:03

kittaakos self-assigned this May 11, 2023

kittaakos force-pushed the PNX-3671 branch 2 times, most recently from d6a4900 to d69174a Compare May 12, 2023 07:03

rhpco approved these changes May 12, 2023

View reviewed changes

per1234 reviewed May 15, 2023

View reviewed changes

kittaakos force-pushed the PNX-3671 branch 2 times, most recently from db14dd8 to e1b3953 Compare May 16, 2023 14:42

kittaakos force-pushed the PNX-3671 branch 2 times, most recently from d3dc5b8 to 96b40c0 Compare May 16, 2023 15:17

per1234 reviewed May 26, 2023

View reviewed changes

arduino-ide-extension/src/node/clang-formatter.ts Outdated Show resolved Hide resolved

kittaakos requested a review from davegarthsimpson May 30, 2023 09:10

davegarthsimpson approved these changes May 30, 2023

View reviewed changes

fix: removed unsafe shell when executing process

a790379

Ref: PNX-3671 Co-authored-by: per1234 <accounts@perglass.com> Co-authored-by: Akos Kitta <a.kitta@arduino.cc> Signed-off-by: Akos Kitta <a.kitta@arduino.cc>

kittaakos force-pushed the PNX-3671 branch from 96b40c0 to a790379 Compare June 5, 2023 07:51

per1234 approved these changes Jun 6, 2023

View reviewed changes

kittaakos merged commit 9d2297c into main Jun 6, 2023

kittaakos deleted the PNX-3671 branch June 6, 2023 09:39

kittaakos mentioned this pull request Jun 23, 2023

Start the CLI daemon process without a shell #2112

Closed

3 tasks

kittaakos pushed a commit that referenced this pull request Aug 20, 2023

fix: incorrect certificate flashing command string

ab9490d

Use a `string` array of command flags instead of the concatenated final `string` command. Ref: #2067 Closes #2179 Signed-off-by: Akos Kitta <a.kitta@arduino.cc>

kittaakos mentioned this pull request Aug 20, 2023

fix: incorrect certificate flashing command string #2181

Merged

4 tasks

kittaakos pushed a commit that referenced this pull request Aug 20, 2023

fix: execute the Arduino CLI without a shell

4398f66

Closes #2112 Ref: #2067 Signed-off-by: Akos Kitta <a.kitta@arduino.cc>

kittaakos mentioned this pull request Aug 20, 2023

fix: execute the Arduino CLI without a shell #2182

Merged

4 tasks

kittaakos pushed a commit that referenced this pull request Aug 20, 2023

fix: execute the Arduino CLI without a shell

2aae9e0

Closes #2112 Ref: #2067 Signed-off-by: Akos Kitta <a.kitta@arduino.cc>

per1234 mentioned this pull request Sep 28, 2023

IDE fails to open: spawn C:\WINDOWS\system32\cmd.exe ENOENT #2073

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: removed unsafe `shell` when executing process #2067

fix: removed unsafe `shell` when executing process #2067

kittaakos commented May 11, 2023 •

edited

Loading

rhpco May 12, 2023

per1234 left a comment

kittaakos commented May 16, 2023

kittaakos commented May 24, 2023

davegarthsimpson left a comment

per1234 left a comment

fix: removed unsafe shell when executing process #2067

fix: removed unsafe shell when executing process #2067

Conversation

kittaakos commented May 11, 2023 • edited Loading

Motivation

Change description

Other information

Reviewer checklist

rhpco May 12, 2023

Choose a reason for hiding this comment

per1234 left a comment

Choose a reason for hiding this comment

Auto Format operation fails

To reproduce

Expected behavior

Arduino IDE version

Operating system

Additional context

kittaakos commented May 16, 2023

kittaakos commented May 24, 2023

davegarthsimpson left a comment

Choose a reason for hiding this comment

per1234 left a comment

Choose a reason for hiding this comment

fix: removed unsafe `shell` when executing process #2067

fix: removed unsafe `shell` when executing process #2067

kittaakos commented May 11, 2023 •

edited

Loading