Skip to content

Path validators reject specifier leading paths #512

Description

@SJrX

Path validators reject specifier-leading paths that systemd accepts

Four value validators require the path to start with a literal /, but systemd expands
%-specifiers before it checks that the path is absolute. Any value that begins with a
specifier is therefore flagged as invalid in the editor on a unit systemd loads fine.

Affected (all use RegexTerminal("/\\S*", "/\\S*")):

Validator Setting(s)
ConfigParseWorkingDirectoryOptionValue WorkingDirectory=
ConfigParseUnitEnvFileOptionValue EnvironmentFile=
ConfigParseUnitMountsForOptionValue RequiresMountsFor=, WantsMountsFor=
ConfigParseExecInputOptionValue StandardInput=file:PATH

Why it's wrong

Every one of these calls unit_path_printf() and only then path_simplify_and_warn(..., PATH_CHECK_ABSOLUTE), so the absolute check runs against the expanded string:

  • config_parse_working_directory — src/core/load-fragment.c
  • config_parse_unit_env_file — src/core/load-fragment.c
  • config_parse_unit_mounts_for — src/core/load-fragment.c
  • config_parse_exec_input — src/core/load-fragment.c (the file: branch)

%S/foo expands to /var/lib/foo, %h/.config/x to /home/user/.config/x, and so on.
The plugin cannot expand specifiers, so it has to accept a leading %X as a possible
absolute path.

Evidence

46 occurrences across a 6,680-unit Debian corpus, all currently flagged. Examples:

  WorkingDirectory=%S/osmocom              x22  (osmo-trx, osmo-cbc, ...)
  WorkingDirectory=%S/labgrid-coordinator  x2
  WorkingDirectory=%E/wireguard
  WorkingDirectory = %h                    x2   (bare specifier, no trailing path)
  EnvironmentFile=-%t/<name>/env_session.conf  x6
  EnvironmentFile=-%h/.config/gpu-screen-recorder.env
  EnvironmentFile=-%E/yubikey-touch-detector/service.conf
  RequiresMountsFor=%f                     x4   (systemd's own systemd-loop@.service)
  RequiresMountsFor=%t/containers          x2

Suggested fix

grammar/Combinators.kt already has the right building blocks from #509. The start rule
is PATH_START = (?:/|%\S), and there are two shapes depending on whether the setting
splits its value:

  • UNIT_PATH — the setting hands the parser the whole rvalue (no splitting, no unquoting,
    no unescaping). Use for WorkingDirectory=, EnvironmentFile= and StandardInput=file:.
  • QUOTABLE_UNIT_PATH — the setting splits with extract_first_word(). Use for
    RequiresMountsFor= / WantsMountsFor=, which pass EXTRACT_UNQUOTE.

Two things to preserve while doing it:

  1. WorkingDirectory= also accepts a bare ~, and an optional leading - before the
    specifier. EnvironmentFile= also takes a leading -, but note systemd strips it from
    the expanded string (path_simplify_and_warn(n[0] == '-' ? n + 1 : n, ...)).
  2. RequiresMountsFor= currently has the same space/quoting bug Symlinks= had before
    More validators #509extract_first_word(..., EXTRACT_UNQUOTE) means "/mnt/My Data" is one valid
    entry. Switching it to QUOTABLE_UNIT_PATH fixes both bugs at once.

Verification

ConditionAndAssertInspectionTest / UnitPathAndUnitNameInspectionTest have the pattern to
copy: assert the specifier forms accepted, relative paths still rejected, and (for
RequiresMountsFor=) quoted entries containing spaces accepted. Run the suite under both
engines — ./gradlew test and ./gradlew test -Dsystemd.unit.grammarParseEngine=true.

All references above are to systemd a8e93919c3, the commit recorded in
systemd-build/build/last_commit_hash.

One thing I'd flag if you're triaging: RequiresMountsFor=%f comes from systemd's own shipped systemd-loop@.service, so this fires on units people actually have installed, not just hand-written ones.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions