Skip to content

Commit 6182cb2

Browse files
committed
fix(installer): pass service password via env, not the exe command line
install-service.ps1 previously appended --password <value> to the exe invocation, exposing the secret on the exe's process command line (readable by any local process via Win32_Process for the install window). It now sets WINDOWS_SERVICE_INSTALL_PASSWORD in the environment and clears it in a finally. win_service.py reads that variable and splices --password into the in-memory arg list handed to pywin32 HandleCommandLine (which parses a Python list and calls CreateService directly), so the password reaches SCM without ever being a process command-line argument. The splice logic lives in a new pywin32-free module, service_install_password, with unit tests for the install/non-install, empty, no-duplication and non-mutation cases. SYNC-461
1 parent ff58b5b commit 6182cb2

4 files changed

Lines changed: 101 additions & 13 deletions

File tree

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import os
2+
from collections.abc import Mapping
3+
from collections.abc import Sequence
4+
5+
# Must match the $env: name set in installer/scripts/install-service.ps1.
6+
SERVICE_INSTALL_PASSWORD_ENV = "WINDOWS_SERVICE_INSTALL_PASSWORD"
7+
8+
_INSTALL_SUBCOMMAND = "install"
9+
10+
11+
def inject_install_password(
12+
install_args: Sequence[str],
13+
env: Mapping[str, str] | None = None,
14+
) -> list[str]:
15+
"""Splice ``--password <value>`` from the environment into an ``install`` arg list.
16+
17+
install-service.ps1 passes the service password via the environment rather than on the
18+
exe's command line (which is readable by any local process for the install window). Here
19+
we read it back and insert it into the in-memory arg list handed to pywin32's
20+
HandleCommandLine, so the secret reaches CreateService without ever appearing in a
21+
process command line.
22+
23+
pywin32 uses POSIX getopt, which requires options before the subcommand, so the flag is
24+
inserted immediately before the ``install`` verb. Any other subcommand, an unset/empty
25+
password, or an arg list that already carries ``--password`` is returned unchanged.
26+
"""
27+
args = list(install_args)
28+
resolved_env = os.environ if env is None else env
29+
password = resolved_env.get(SERVICE_INSTALL_PASSWORD_ENV)
30+
if not password:
31+
return args
32+
if _INSTALL_SUBCOMMAND not in args:
33+
return args
34+
if "--password" in args:
35+
return args
36+
idx = args.index(_INSTALL_SUBCOMMAND)
37+
return [*args[:idx], "--password", password, *args[idx:]]

template/{% if has_backend %}backend{% endif %}/src/backend_api/entrypoint/{% if install_as_windows_service %}win_service.py{% endif %}

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ from ..jinja_constants import APP_NAME
1919
from ..jinja_constants import WINDOWS_SERVICE_DISPLAY_NAME
2020
from ..jinja_constants import WINDOWS_SERVICE_NAME
2121
from .parser import parser
22+
from .service_install_password import inject_install_password
2223

2324
CRASH_DUMP_FILENAME = "service-crash.log"
2425
_SERVICE_MANAGEMENT_COMMANDS = frozenset({"install", "start", "stop", "remove", "debug"})
@@ -134,6 +135,10 @@ def dispatch_windows_service(argv: Sequence[str]) -> int:
134135
# pywin32 reads _exe_args_ at install time and bakes the suffix into the ImagePath
135136
# registry value itself — no post-install registry patching needed.
136137
AppService._exe_args_ = subprocess.list2cmdline(["service", *runtime_args])
138+
# The service password arrives via the environment, not the command line (which any
139+
# local process can read); splice it into the in-memory arg list here so it reaches
140+
# CreateService without ever appearing in the exe's process command line.
141+
install_args = inject_install_password(install_args)
137142
win32serviceutil.HandleCommandLine(AppService, argv=[sys.argv[0], *install_args])
138143
else:
139144
servicemanager.Initialize()
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import pytest
2+
from backend_api.entrypoint.service_install_password import SERVICE_INSTALL_PASSWORD_ENV
3+
from backend_api.entrypoint.service_install_password import inject_install_password
4+
5+
6+
def test_Given_no_password_env_var__Then_args_unchanged():
7+
args = ["--username", ".\\svc", "--startup", "auto", "install"]
8+
9+
assert inject_install_password(args, env={}) == args
10+
11+
12+
def test_Given_password_env_var__Then_spliced_before_install_subcommand():
13+
# pywin32 uses POSIX getopt, so options must come before the subcommand.
14+
args = ["--username", ".\\svc", "--startup", "auto", "install"]
15+
16+
result = inject_install_password(args, env={SERVICE_INSTALL_PASSWORD_ENV: "s3cret"})
17+
18+
assert result == ["--username", ".\\svc", "--startup", "auto", "--password", "s3cret", "install"]
19+
20+
21+
@pytest.mark.parametrize("verb", ["start", "stop", "remove", "debug"])
22+
def test_Given_non_install_subcommand__Then_password_not_injected(verb: str):
23+
args = [verb]
24+
25+
assert inject_install_password(args, env={SERVICE_INSTALL_PASSWORD_ENV: "s3cret"}) == args
26+
27+
28+
def test_Given_empty_password__Then_treated_as_unset():
29+
args = ["install"]
30+
31+
assert inject_install_password(args, env={SERVICE_INSTALL_PASSWORD_ENV: ""}) == args
32+
33+
34+
def test_Given_password_already_present__Then_not_duplicated():
35+
args = ["--username", ".\\svc", "--password", "already", "install"]
36+
37+
assert inject_install_password(args, env={SERVICE_INSTALL_PASSWORD_ENV: "s3cret"}) == args
38+
39+
40+
def test_Given_call__Then_input_list_not_mutated():
41+
args = ["install"]
42+
43+
_ = inject_install_password(args, env={SERVICE_INSTALL_PASSWORD_ENV: "s3cret"})
44+
45+
assert args == ["install"]

template/{% if install_as_windows_service %}installer{% endif %}/scripts/install-service.ps1.jinja

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -55,24 +55,25 @@ if ($ServiceAccount -ieq 'custom') {
5555
& icacls $LogDir /grant "${rightsAccount}:(OI)(CI)M" /T | Out-Null
5656
if ($LASTEXITCODE -ne 0) { throw "icacls failed to grant Modify on $LogDir for $rightsAccount (exit $LASTEXITCODE)" }
5757

58-
$installArgs += @('--username', $account, '--password', $Password)
58+
# The password is NOT passed on the command line: the exe's process command line is
59+
# readable by any local process (Win32_Process) for the install window. It travels via
60+
# an environment variable instead, which win_service.py reads and splices into pywin32's
61+
# in-memory HandleCommandLine arg list. The var name must match SERVICE_INSTALL_PASSWORD_ENV
62+
# in backend_api.entrypoint.service_install_password.
63+
$installArgs += @('--username', $account)
64+
$env:WINDOWS_SERVICE_INSTALL_PASSWORD = $Password
5965
}
6066

6167
$installArgs += @('--startup', 'auto', 'install', '--', '--port', $Port, '--log-folder', $LogDir)
6268

63-
$logArgs = @()
64-
$maskNext = $false
65-
foreach ($arg in $installArgs) {
66-
if ($maskNext) {
67-
$logArgs += '***'
68-
$maskNext = $false
69-
continue
70-
}
71-
$logArgs += $arg
72-
if ($arg -eq '--password') { $maskNext = $true }
69+
# $installArgs no longer carries the password, so it is safe to log verbatim.
70+
Write-Host "Installing service: $ExePath $($installArgs -join ' ')"
71+
try {
72+
& $ExePath @installArgs
73+
}
74+
finally {
75+
Remove-Item Env:\WINDOWS_SERVICE_INSTALL_PASSWORD -ErrorAction SilentlyContinue
7376
}
74-
Write-Host "Installing service: $ExePath $($logArgs -join ' ')"
75-
& $ExePath @installArgs
7677
if ($LASTEXITCODE -ne 0) { throw "Service install failed (exit $LASTEXITCODE)" }
7778

7879
Write-Host "Starting service $ServiceName"

0 commit comments

Comments
 (0)