Let command plugins ask for network permissions by neonichu · Pull Request #6114 · swiftlang/swift-package-manager

neonichu · 2023-02-03T02:25:14Z

This adds a new plugin permission that allows a command plugin to ask for networking permissions. The permission can distinguish between local and outgoing connections, as well as specifying a list or range of ports to allow. Similar to existing permissions, there's also a CLI option for allowing connections.

resolves #5489

neonichu · 2023-02-03T02:25:26Z

@swift-ci please smoke test

neonichu · 2023-02-03T20:44:48Z

@swift-ci please smoke test

neonichu · 2023-02-03T20:54:50Z

+                    let answer = readLine(strippingNewline: true)
+                    // Throw an error if we didn't get permission.
+                    if answer?.lowercased() != "yes" {
+                        throw StringError("Plugin was denied permission to \(permissionString).")


Note: I changed these to StringError to avoid having usage be printed to the console which seems to be a behavior of ArgumentParser's ValidationError.

tomerd

this is great. consider adding a few more tests for different manifest usage patterns

neonichu · 2023-02-03T22:49:38Z

@swift-ci please smoke test

neonichu · 2023-02-04T00:24:05Z

@swift-ci please smoke test

neonichu · 2023-02-04T00:56:36Z

@swift-ci please smoke test

tomerd · 2023-02-04T02:21:13Z

                    accessibleTools: accessibleTools,
                    writableDirectories: writableDirectories,
                    readOnlyDirectories: readOnlyDirectories,
+                    allowNetworkConnections: [],


not sure I follow why this is empty in this call?

This is the invocation for build tools which do not support specifying permissions.

tomerd · 2023-02-04T02:38:27Z

cc @fabianfett

This adds a new plugin permission that allows a command plugin to ask for networking permissions. The permission can distinguish between local and outgoing connections, as well as specifying a list or range of ports to allow. Similar to existing permissions, there's also a CLI option for allowing connections. resolves #5489

neonichu · 2023-02-06T17:11:16Z

@swift-ci please smoke test

neonichu · 2023-02-06T18:59:03Z

@swift-ci please smoke test

neonichu · 2023-02-06T21:48:05Z

Windows failure looks like something in llbuild? cc @compnerd

[143/150][ 95%][23.201s] Linking CXX executable bin\CAPITests.exe
FAILED: bin/CAPITests.exe 
cmd.exe /C "cd . && "C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe" -E vs_link_exe --intdir=unittests\CAPI\CMakeFiles\CAPITests.dir --rc=C:\PROGRA~2\WI3CF2~1\10\bin\100177~1.0\x64\rc.exe --mt=C:\PROGRA~2\WI3CF2~1\10\bin\100177~1.0\x64\mt.exe --manifests  -- T:\1\bin\lld-link.exe /nologo unittests\CAPI\CMakeFiles\CAPITests.dir\C-API.cpp.obj unittests\CAPI\CMakeFiles\CAPITests.dir\BuildSystem-C-API.cpp.obj  /out:bin\CAPITests.exe /implib:lib\CAPITests.lib /pdb:bin\CAPITests.pdb /version:0.0 /INCREMENTAL:NO /INCREMENTAL:NO /subsystem:console  lib\gtest.lib  lib\gtest_main.lib  lib\llvmSupport.lib  lib\llbuildBasic.lib  lib\llbuild.lib  T:\Library\sqlite-3.36.0\usr\lib\SQLite3.lib  lib\gtest.lib  lib\llvmSupport.lib  lib\LLVMDemangle.lib  ShLwApi.lib  kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib && cd ."
LINK: command "T:\1\bin\lld-link.exe /nologo unittests\CAPI\CMakeFiles\CAPITests.dir\C-API.cpp.obj unittests\CAPI\CMakeFiles\CAPITests.dir\BuildSystem-C-API.cpp.obj /out:bin\CAPITests.exe /implib:lib\CAPITests.lib /pdb:bin\CAPITests.pdb /version:0.0 /INCREMENTAL:NO /INCREMENTAL:NO /subsystem:console lib\gtest.lib lib\gtest_main.lib lib\llvmSupport.lib lib\llbuildBasic.lib lib\llbuild.lib T:\Library\sqlite-3.36.0\usr\lib\SQLite3.lib lib\gtest.lib lib\llvmSupport.lib lib\LLVMDemangle.lib ShLwApi.lib kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib /MANIFEST /MANIFESTFILE:bin\CAPITests.exe.manifest" failed (exit code 1) with the following output:
lld-link: error: undefined symbol: llb_get_api_version
>>> referenced by unittests\CAPI\CMakeFiles\CAPITests.dir\C-API.cpp.obj:(private: virtual void __cdecl `anonymous namespace'::CAPI_GetAPIVersion_Test::TestBody(void))

compnerd · 2023-02-06T22:15:55Z

Please test with following PRs:
swiftlang/swift-llbuild#860

@swift-ci please test Windows platform

neonichu · 2023-02-07T18:46:08Z

@swift-ci please test Windows platform

neonichu · 2023-02-07T20:31:05Z

Seems like update_checkout is broken on Windows right now cc @shahmishal @compnerd

[llvm-project]                          Updating 'C:\Users\swift-ci\jenkins\workspace\swiftpm-PR-windows\llvm-project'
Error on repo "C:\Users\swift-ci\jenkins\workspace\swiftpm-PR-windows\llvm-project": Traceback (most recent call last):
  File "C:\Users\swift-ci\jenkins\workspace\swiftpm-PR-windows\swift\utils\update_checkout\update_checkout\update_checkout.py", line 201, in update_single_repository
    shell.run(['git', 'rev-parse', '--verify', checkout_target])
  File "C:\Users\swift-ci\jenkins\workspace\swiftpm-PR-windows\swift\utils\swift_build_support\swift_build_support\shell.py", line 255, in run
    raise eout
Exception: ['git', 'rev-parse', '--verify', 'stable/20220421']

There are also more follow-on errors.

neonichu · 2023-02-08T00:10:12Z

Hm, I guess the Windows CI re-run itself, confusing...

shahmishal · 2023-02-08T00:23:12Z

Hm, I guess the Windows CI re-run itself, confusing...

I did it from the backend, sorry I should have told you about it.

MaxDesiatov · 2023-02-08T17:32:59Z

 ```

-Command plugins that need to write to the file system will cause SwiftPM to ask the user for approval if `swift package` is invoked from a console, or deny the request if it is not.  Passing the `--allow-writing-to-package-directory` flag to the `swift package` invocation will allow the request without questions — this is particularly useful in a Continuous Integration environment.
+Command plugins that need to write to the file system will cause SwiftPM to ask the user for approval if `swift package` is invoked from a console, or deny the request if it is not.  Passing the `--allow-writing-to-package-directory` flag to the `swift package` invocation will allow the request without questions — this is particularly useful in a Continuous Integration environment. Similarly, the `--allow-network-connections` flag can be used to allow network connections without showing a prompt.


Should this change also be mentioned in the changelog?

+1 this is a great feature we should mention in chnagelog and release notes

Added in #6132

neonichu self-assigned this Feb 3, 2023

neonichu force-pushed the command-plugin-network-permissions branch 2 times, most recently from 54ccb20 to 736e565 Compare February 3, 2023 20:43

neonichu changed the title ~~WIP: Let command plugins ask for network permissions~~ Let command plugins ask for network permissions Feb 3, 2023

neonichu marked this pull request as ready for review February 3, 2023 20:44

neonichu requested review from abertelrud, elsh and tomerd as code owners February 3, 2023 20:44

neonichu commented Feb 3, 2023

View reviewed changes

tomerd reviewed Feb 3, 2023

View reviewed changes

Comment thread Sources/Workspace/DefaultPluginScriptRunner.swift Outdated

tomerd reviewed Feb 3, 2023

View reviewed changes

Comment thread Sources/PackageDescription/Target.swift

tomerd approved these changes Feb 3, 2023

View reviewed changes

neonichu force-pushed the command-plugin-network-permissions branch from 736e565 to ce08da5 Compare February 3, 2023 22:48

neonichu force-pushed the command-plugin-network-permissions branch from ce08da5 to ac28899 Compare February 4, 2023 00:23

neonichu force-pushed the command-plugin-network-permissions branch from ac28899 to dcf0a19 Compare February 4, 2023 00:56

tomerd reviewed Feb 4, 2023

View reviewed changes

Comment thread Sources/Commands/PackageTools/PluginCommand.swift Outdated

tomerd reviewed Feb 4, 2023

View reviewed changes

Comment thread Sources/Workspace/DefaultPluginScriptRunner.swift Outdated

tomerd reviewed Feb 4, 2023

View reviewed changes

Comment thread Tests/CommandsTests/PackageToolTests.swift

stevapple reviewed Feb 5, 2023

View reviewed changes

Comment thread Documentation/Plugins.md Outdated

neonichu force-pushed the command-plugin-network-permissions branch from 32a44a3 to 83d7b3d Compare February 6, 2023 17:10

neonichu merged commit 00ac2ad into main Feb 8, 2023

neonichu deleted the command-plugin-network-permissions branch February 8, 2023 00:10

neonichu mentioned this pull request Feb 8, 2023

update plugin argument parsing so that execution arguments can be passed after command plugin name #6128

Merged

MaxDesiatov reviewed Feb 8, 2023

View reviewed changes

carson-katri mentioned this pull request May 2, 2023

Use a Plugin for the TutorialRepoGenerator liveview-native/liveview-client-swiftui#898

Closed

Uh oh!

Conversation

neonichu commented Feb 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

neonichu commented Feb 3, 2023

Uh oh!

neonichu commented Feb 3, 2023

Uh oh!

neonichu Feb 3, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tomerd left a comment

Choose a reason for hiding this comment

Uh oh!

neonichu commented Feb 3, 2023

Uh oh!

neonichu commented Feb 4, 2023

Uh oh!

neonichu commented Feb 4, 2023

Uh oh!

Uh oh!

tomerd Feb 4, 2023

Choose a reason for hiding this comment

Uh oh!

neonichu Feb 6, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tomerd commented Feb 4, 2023

Uh oh!

Uh oh!

neonichu commented Feb 6, 2023

Uh oh!

neonichu commented Feb 6, 2023

Uh oh!

neonichu commented Feb 6, 2023

Uh oh!

compnerd commented Feb 6, 2023

Uh oh!

neonichu commented Feb 7, 2023

Uh oh!

neonichu commented Feb 7, 2023

Uh oh!

neonichu commented Feb 8, 2023

Uh oh!

shahmishal commented Feb 8, 2023

Uh oh!

MaxDesiatov Feb 8, 2023

Choose a reason for hiding this comment

Uh oh!

tomerd Feb 8, 2023

Choose a reason for hiding this comment

Uh oh!

neonichu Feb 8, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

neonichu commented Feb 3, 2023 •

edited

Loading