diff --git a/src/main/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/semanticdata/optionvalues/ExecOptionValue.kt b/src/main/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/semanticdata/optionvalues/ExecOptionValue.kt index 820c7cf..12dd841 100644 --- a/src/main/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/semanticdata/optionvalues/ExecOptionValue.kt +++ b/src/main/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/semanticdata/optionvalues/ExecOptionValue.kt @@ -51,8 +51,15 @@ class ExecOptionValue : OptionValueInformation { get() = VALIDATOR_NAME companion object { - // Used to check whether or not there is a problem. - private val ABSOLUTE_PATH_REGEX = Pattern.compile("^\\s*(?:[+@:!-]|!!)*\\s*[\\/]") + // Specifiers that systemd's unit_path_printf() expands to an absolute filesystem path. A command whose first + // token begins with one of these (e.g. ExecStart=%h/bin/foo) resolves to an absolute path, so it must not be + // flagged by the "use an absolute path" recommendation below. Text/name specifiers (%i, %n, %H, ...) are + // deliberately excluded: they do not guarantee an absolute path. See src/core/unit-printf.c in systemd. + private const val ABSOLUTE_PATH_SPECIFIERS = "hsCdDELStTVyYf" + + // Used to check whether or not there is a problem. A value is considered absolute when, after the optional exec + // prefixes (+ @ : ! -), it starts with '/' or with a specifier that expands to an absolute path. + private val ABSOLUTE_PATH_REGEX = Pattern.compile("^\\s*(?:[+@:!-]|!!)*\\s*(?:[\\/]|%[$ABSOLUTE_PATH_SPECIFIERS])") // Used to determine what to highlight (we want to avoid highlighting valid prefixes). private val RELATIVE_PATH_REGEX = Pattern.compile("^\\s*(?:[+@:!-]+)?([^\\/\\s]\\S*)\\s*") diff --git a/src/test/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/inspections/InvalidValueInspectionForExecOptionsTest.kt b/src/test/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/inspections/InvalidValueInspectionForExecOptionsTest.kt index e759d0e..8aac1f1 100644 --- a/src/test/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/inspections/InvalidValueInspectionForExecOptionsTest.kt +++ b/src/test/kotlin/net/sjrx/intellij/plugins/systemdunitfiles/inspections/InvalidValueInspectionForExecOptionsTest.kt @@ -338,4 +338,154 @@ class InvalidValueInspectionForExecOptionsTest : AbstractUnitFileTest() { assertStringContains("an absolute path", info!!.description) TestCase.assertEquals("docker-compose", info.text) } + + fun testNoWarningWhenUsingHomeDirectorySpecifier() { + // Regression test for GitHub issue #529: %h is a valid systemd specifier that expands to the user's home + // directory (an absolute path), so ExecStart=%h/... must not be flagged. + // Fixture Setup + val file = """ + [Service] + ExecStart=%h/path/to/executable + + """.trimIndent() + + + // Execute SUT + setupFileInEditor("file.service", file) + enableInspection(InvalidValueInspection::class.java) + val highlights = myFixture.doHighlighting() + + // Verification + assertSize(0, highlights) + } + + fun testNoWarningWhenUsingRuntimeDirectorySpecifierWithDashPrefix() { + // %t expands to the runtime directory root (an absolute path); the dash prefix must not change that. + // Fixture Setup + val file = """ + [Service] + ExecStart=-%t/foo/bar + + """.trimIndent() + + + // Execute SUT + setupFileInEditor("file.service", file) + enableInspection(InvalidValueInspection::class.java) + val highlights = myFixture.doHighlighting() + + // Verification + assertSize(0, highlights) + } + + fun testNoWarningWhenUsingUserShellSpecifier() { + // %s expands to the user's shell (an absolute path such as /bin/bash). + // Fixture Setup + val file = """ + [Service] + ExecStart=%s + + """.trimIndent() + + + // Execute SUT + setupFileInEditor("file.service", file) + enableInspection(InvalidValueInspection::class.java) + val highlights = myFixture.doHighlighting() + + // Verification + assertSize(0, highlights) + } + + fun testWeakWarningWhenUsingNonAbsolutePathSpecifier() { + // %n (the unit name) is NOT guaranteed to be an absolute path, so the recommendation should still fire. + // Fixture Setup + val file = """ + [Service] + ExecStart=%n/path/to/executable + + """.trimIndent() + + + // Execute SUT + setupFileInEditor("file.service", file) + enableInspection(InvalidValueInspection::class.java) + val highlights = myFixture.doHighlighting() + + // Verification + assertSize(1, highlights) + val info = highlights[0] + assertStringContains("an absolute path", info!!.description) + TestCase.assertEquals("%n/path/to/executable", info.text) + } + + fun testNoWarningForAnyAbsolutePathSpecifier() { + // Every specifier that systemd's unit_path_printf() expands to an absolute path must suppress the warning. + // These are the full whitelist from ExecOptionValue.ABSOLUTE_PATH_SPECIFIERS (h s C d D E L S t T V y Y f); + // keep the two in sync. + // Fixture Setup + // language="unit file (systemd)" + val file = """ + [Service] + ExecStart=%h/path/to/executable + ExecStart=%s/path/to/executable + ExecStart=%C/path/to/executable + ExecStart=%d/path/to/executable + ExecStart=%D/path/to/executable + ExecStart=%E/path/to/executable + ExecStart=%L/path/to/executable + ExecStart=%S/path/to/executable + ExecStart=%t/path/to/executable + ExecStart=%T/path/to/executable + ExecStart=%V/path/to/executable + ExecStart=%y/path/to/executable + ExecStart=%Y/path/to/executable + ExecStart=%f/path/to/executable + + """.trimIndent() + + + // Execute SUT + setupFileInEditor("file.service", file) + enableInspection(InvalidValueInspection::class.java) + val highlights = myFixture.doHighlighting() + + // Verification + assertSize(0, highlights) + } + + fun testWeakWarningForNonAbsolutePathSpecifiers() { + // Specifiers that expand to arbitrary text (names, host/OS facts, user/group) are NOT absolute paths, so the + // recommendation must still fire once per line. None of these appears in ExecOptionValue.ABSOLUTE_PATH_SPECIFIERS. + // Fixture Setup + // language="unit file (systemd)" + val file = """ + [Service] + ExecStart=%i/path/to/executable + ExecStart=%I/path/to/executable + ExecStart=%n/path/to/executable + ExecStart=%N/path/to/executable + ExecStart=%p/path/to/executable + ExecStart=%P/path/to/executable + ExecStart=%H/path/to/executable + ExecStart=%l/path/to/executable + ExecStart=%m/path/to/executable + ExecStart=%M/path/to/executable + ExecStart=%u/path/to/executable + ExecStart=%U/path/to/executable + ExecStart=%a/path/to/executable + ExecStart=%b/path/to/executable + ExecStart=%v/path/to/executable + + """.trimIndent() + + + // Execute SUT + setupFileInEditor("file.service", file) + enableInspection(InvalidValueInspection::class.java) + val highlights = myFixture.doHighlighting() + + // Verification + assertSize(15, highlights) + } }