[Security] Harden module installer shell command hooks #232

New Issue

zemion · 2026-07-11T11:37:37+02:00

zemion commented

2026-07-11 11:37:37 +02:00

Problem

The module installer executes restart and external database backup/restore commands via subprocess.run(..., shell=True). These hooks may be intentional operator functionality, but if command strings can be influenced through admin/config/package paths, this becomes command execution.

Observed code paths:

src/govoplan_core/core/module_installer.py:600 accepts restart command strings.
src/govoplan_core/core/module_installer.py:2609 executes restart commands with shell=True.
src/govoplan_core/core/module_installer.py:3005 stores and runs external DB backup/restore commands.
src/govoplan_core/core/module_installer.py:3148 executes DB hooks with shell=True.

Acceptance Criteria

Prefer argv arrays over shell strings for restart/backup/restore hooks.
If shell strings remain supported, require an explicit operator-local setting and block them from API/package-controlled paths.
Add allowlist or command-profile support for production installations.
Record clear audit entries whenever a hook command is executed.
Tests cover rejection of shell metacharacter injection in non-operator paths.

## Problem The module installer executes restart and external database backup/restore commands via `subprocess.run(..., shell=True)`. These hooks may be intentional operator functionality, but if command strings can be influenced through admin/config/package paths, this becomes command execution. Observed code paths: - `src/govoplan_core/core/module_installer.py:600` accepts restart command strings. - `src/govoplan_core/core/module_installer.py:2609` executes restart commands with `shell=True`. - `src/govoplan_core/core/module_installer.py:3005` stores and runs external DB backup/restore commands. - `src/govoplan_core/core/module_installer.py:3148` executes DB hooks with `shell=True`. ## Acceptance Criteria - Prefer argv arrays over shell strings for restart/backup/restore hooks. - If shell strings remain supported, require an explicit operator-local setting and block them from API/package-controlled paths. - Add allowlist or command-profile support for production installations. - Record clear audit entries whenever a hook command is executed. - Tests cover rejection of shell metacharacter injection in non-operator paths.

zemion added the

labels 2026-07-11 11:37:37 +02:00

zemion commented

2026-07-11 12:51:47 +02:00

Full audit confirmation from audit-reports/govoplan-full-20260711-1238:

govoplan-core/src/govoplan_core/core/module_installer.py:2614 restart command uses subprocess.run(..., shell=True).
govoplan-core/src/govoplan_core/core/module_installer.py:3224 database backup/restore hook uses subprocess.run(..., shell=True).
Bandit reports B602 high; Semgrep reports subprocess-shell-true.

I accidentally created duplicate govoplan-core#237 while importing the grouped audit issues; that duplicate is now closed in favor of this existing issue.

Full audit confirmation from `audit-reports/govoplan-full-20260711-1238`: - `govoplan-core/src/govoplan_core/core/module_installer.py:2614` restart command uses `subprocess.run(..., shell=True)`. - `govoplan-core/src/govoplan_core/core/module_installer.py:3224` database backup/restore hook uses `subprocess.run(..., shell=True)`. - Bandit reports `B602` high; Semgrep reports `subprocess-shell-true`. I accidentally created duplicate `govoplan-core#237` while importing the grouped audit issues; that duplicate is now closed in favor of this existing issue.

zemion added the

area/security

audit/structural

source/security-audit

labels 2026-07-11 16:03:22 +02:00

zemion referenced this issue from add-ideas/govoplan

2026-07-11 16:14:03 +02:00

[Audit] Burndown: 2026-07-11 full GovOPlaN security and quality report #4

zemion commented

2026-07-11 17:38:54 +02:00

Codex State: done

Summary

Replaced module installer restart and database hook shell=True execution with argv execution via shlex.split.
Shell operators, redirects, substitutions, and inline environment-assignment shell syntax are now rejected instead of interpreted.
Missing commands now return a controlled 127 result instead of raising through the installer flow.

Changed Files

src/govoplan_core/core/module_installer.py

Verification

python3 -m py_compile src/govoplan_core/core/module_installer.py ...
direct smoke: python3 -c "print(42)" hook succeeds; echo hello > /tmp/unsafe is rejected with return code 2; missing command returns 127.

Local fix is ready to push.

## Codex State: done ### Summary - Replaced module installer restart and database hook `shell=True` execution with argv execution via `shlex.split`. - Shell operators, redirects, substitutions, and inline environment-assignment shell syntax are now rejected instead of interpreted. - Missing commands now return a controlled `127` result instead of raising through the installer flow. ### Changed Files - `src/govoplan_core/core/module_installer.py` ### Verification - `python3 -m py_compile src/govoplan_core/core/module_installer.py ...` - direct smoke: `python3 -c "print(42)"` hook succeeds; `echo hello > /tmp/unsafe` is rejected with return code `2`; missing command returns `127`. Local fix is ready to push.

zemion closed this issue

2026-07-11 17:39:03 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: add-ideas/govoplan-core#232