Fix LaunchAgents directory permissions, bail if `-install` is run with sudo #244

nonrational · 2020-04-24T15:58:44Z

Resolves #141
Resolves #76

The permission bits used to create ~/Library/LaunchAgents (if it doesn't exist) are incorrectly set to 0644 instead of 0755. This PR correctly sets it to 0755.
Since I was already touching setup, decided to knock out puma-dev -install should warn if run under sudo #76 as well. If you try to run puma-dev install under sudo, we bail out immediately.

Changes

Change 0644 to 0755 @ (will_provide_link)
To enable testing, refactor InstallIntoSystem to take a struct for all its arguments.
Add setup_test.go to verify that InstallIntoSystem creates the directory with the correct permissions.
Given we can't test the "live" code without blowing away the ~/Library/LaunchAgents directory locally, write a makefile test that does the assets for us locally.
~~Write a test that ONLY runs on TravisCI machines that actually exercises the -setup main path. Not sure how that will go...~~ Punting on this. It's hairy.
Ancillary: Limit DNS retry attempts to 3 instead of 10 to enable faster failure.
Ancillary: Add golintci-lint config file
Refuse to allow -install to run with sudo.

nonrational · 2020-04-24T17:32:11Z

dev/setup_darwin.go

-	return nil
-}
-
-func Stop() error {


moved this to dev/stop.go, since it's used in both linux and darwin (and doesn't have anything to do with setup)

nonrational · 2020-04-24T17:33:49Z

dev/setup_darwin.go

+	LaunchAgentDirPath string
+	Domains            string
+	Timeout            string
+}


provide args type to approximate named function params.

nonrational · 2020-04-24T17:34:27Z

dev/dns_test.go

@@ -44,6 +44,7 @@ func TestServeDNS(t *testing.T) {

 				return nil
 			},
+			retry.Attempts(3),


limit retries to 3 in tests to help fail faster. added while debugging another issue.

nonrational · 2020-04-24T17:35:35Z

cmd/puma-dev/main_darwin.go

+			Domains:            *fDomains,
+			LaunchAgentDirPath: "~/Library/LaunchAgents",
+			ListenPort:         *fInstallPort,
+			LogfilePath:        "~/Library/Logs/puma-dev.log",


probably a good idea to pull this out into shared variable, then add something like puma-dev -f logs (similar to heroku -f logs)

again, coming in follow-up PR.

* origin/master: GitHub Actions CI (#245) Improve word choice (#243) Add proxy/HTTPS example for Rails and Webpack Dev Server (#242)

nonrational · 2020-04-29T16:56:59Z

.github/workflows/ci.yml

-      - uses: actions/upload-artifact@v1
-        with:
-          name: go-${{ matrix.go-version }}-${{ matrix.os }}
-          path: coverage.html


given that this uploads to a zip, kinda useless.

nonrational · 2020-04-29T16:57:33Z

.golangci.yml

+  timeout: 1m
+  tests: true
+service:
+  golangci-lint-version: 1.24.x # use the fixed version to not introduce new linters unexpectedly


dropping this here to potentially include in CI in future.

nonrational · 2020-04-29T16:59:03Z

Makefile

+	./puma-dev -d 'test:localhost:loc.al:puma' -install
+	test -f "$$HOME/Library/LaunchAgents/io.puma.dev.plist"
+	sleep 2
+	test -f "$$HOME/Library/Logs/puma-dev.log"


since we can't test the "default" install path inside unit tests, provide this helper to guarantee that files exist as expected.

i have a follow-up PR that reworks this a bit.

nonrational · 2020-04-29T16:59:24Z

cmd/puma-dev/main_darwin.go

+			Domains:            *fDomains,
+			LaunchAgentDirPath: "~/Library/LaunchAgents",
+			ListenPort:         *fInstallPort,
+			LogfilePath:        "~/Library/Logs/puma-dev.log",


again, coming in follow-up PR.

nonrational · 2020-04-29T16:59:47Z

dev/setup_darwin.go

@@ -95,7 +95,21 @@ func Cleanup() {
 	}
 }

-func InstallIntoSystem(listenPort, tlsPort int, dir, domains, timeout string) error {
+type InstallIntoSystemArgs struct {


not sure if convention is to call this -Args or -Params or something else...

nonrational · 2020-04-29T17:00:07Z

dev/setup_darwin.go

-	plist := homedir.MustExpand("~/Library/LaunchAgents/io.puma.dev.plist")
-
-	err = os.MkdirAll(plistDir, 0644)
+	err = os.MkdirAll(plistDir, 0755)


this is the bugfix.

nonrational added 10 commits April 23, 2020 12:50

refactor to use args struct so we can override whatever we want

052fc1b

add failing test

e24b4c4

limit retries, fix directory perms

608c49b

finish setup and teardown for setup test

af8cd33

remove devtest

39e9fb4

add makefile script to ensure that we're using the correct defaults

03f1c00

use test-macos-interactive prefix

9295914

stub for golangci config

fdb9962

tidy

e8ef808

no need to update this

99c6cfa

nonrational requested a review from evanphx April 24, 2020 15:58

nonrational added 3 commits April 24, 2020 12:07

rename files to clarify that they're only to be used on darwin arch

6309adb

wonderful to have the opportunity to write stop.go

41a101d

ensure that we don't try to trust keys during setup

ce51956

nonrational commented Apr 24, 2020

View reviewed changes

nonrational added 2 commits April 24, 2020 13:40

add newline

d2445fa

fail if trying to -install as root

dd155d0

nonrational changed the title ~~Fix LaunchAgents directory permissions~~ Fix LaunchAgents directory permissions, bail if -install is run with sudo Apr 24, 2020

nonrational added 6 commits April 27, 2020 17:33

cleanup makefile

302e07e

Merge remote-tracking branch 'origin/master' into launch-agent-perms

9408361

* origin/master: GitHub Actions CI (#245) Improve word choice (#243) Add proxy/HTTPS example for Rails and Webpack Dev Server (#242)

upload all coverage files

e1377fe

use v2 of artifact

559be0b

remove artifact zip for now

ba49792

remove blank line

ea74abb

nonrational commented Apr 29, 2020

View reviewed changes

evanphx approved these changes Apr 29, 2020

View reviewed changes

nonrational merged commit e264766 into master Apr 29, 2020

nonrational deleted the launch-agent-perms branch September 19, 2020 17:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix LaunchAgents directory permissions, bail if `-install` is run with sudo #244

Fix LaunchAgents directory permissions, bail if `-install` is run with sudo #244

nonrational commented Apr 24, 2020 •

edited

Loading

nonrational Apr 24, 2020 •

edited

Loading

nonrational Apr 24, 2020

nonrational Apr 24, 2020

nonrational Apr 24, 2020

nonrational Apr 29, 2020

nonrational Apr 29, 2020

nonrational Apr 29, 2020

nonrational Apr 29, 2020

nonrational Apr 29, 2020

nonrational Apr 29, 2020

nonrational Apr 29, 2020

Fix LaunchAgents directory permissions, bail if -install is run with sudo #244

Fix LaunchAgents directory permissions, bail if -install is run with sudo #244

Conversation

nonrational commented Apr 24, 2020 • edited Loading

Changes

nonrational Apr 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fix LaunchAgents directory permissions, bail if `-install` is run with sudo #244

Fix LaunchAgents directory permissions, bail if `-install` is run with sudo #244

nonrational commented Apr 24, 2020 •

edited

Loading

nonrational Apr 24, 2020 •

edited

Loading