Skip to content

Commit b5b4cdc

Browse files
Merge pull request #1249 from GitGuardian/ctourriere/fix_root_plugin_installation
Ctourriere/fix root plugin installation
2 parents 43faad3 + 44dfb2a commit b5b4cdc

10 files changed

Lines changed: 732 additions & 84 deletions

File tree

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
### Fixed
2+
3+
- Plugin installs and updates now enable the canonical `ggshield.plugins` entry point instead of the wheel package name, migrating any pre-existing alias row (and preserving its `auto_update` setting), and local plugin wheels extract into the active runtime cache so mixed root/admin and user executions do not silently lose registered commands.
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
### Fixed
2+
3+
- ggshield now prunes stale extracted plugin wheel caches during plugin load and removes a plugin's extracted cache on uninstall, preventing old wheel versions from accumulating in the cache directory.

ggshield/cmd/plugin/install.py

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
InsecureSourceError,
2727
PluginDownloader,
2828
)
29-
from ggshield.core.plugin.loader import resolve_config_key
29+
from ggshield.core.plugin.loader import enable_installed_plugin
3030
from ggshield.core.plugin.platform import get_platform_info
3131
from ggshield.core.plugin.signature import (
3232
SignatureVerificationError,
@@ -229,10 +229,11 @@ def _install_from_gitguardian(
229229
bundle_bytes=bundle_bytes,
230230
)
231231

232-
config_key = resolve_config_key(wheel_path, fallback=plugin_name)
233-
enterprise_config.enable_plugin(config_key, version=info.version)
232+
installed_name = enable_installed_plugin(
233+
enterprise_config, plugin_name, info.version, wheel_path
234+
)
234235
enterprise_config.save()
235-
ui.display_info(f"Installed {plugin_name} v{info.version}")
236+
ui.display_info(f"Installed {installed_name} v{info.version}")
236237

237238
except SignatureVerificationError as e:
238239
ui.display_error(f"Signature verification failed for {plugin_name}: {e}")
@@ -273,8 +274,9 @@ def _install_from_local_wheel(
273274
wheel_path, signature_mode=signature_mode
274275
)
275276

276-
config_key = resolve_config_key(installed_wheel, fallback=plugin_name)
277-
enterprise_config.enable_plugin(config_key, version=version)
277+
plugin_name = enable_installed_plugin(
278+
enterprise_config, plugin_name, version, installed_wheel
279+
)
278280
enterprise_config.save()
279281

280282
ui.display_info(f"Installed {plugin_name} v{version}")
@@ -309,8 +311,9 @@ def _install_from_url(
309311
url, sha256, signature_mode=signature_mode
310312
)
311313

312-
config_key = resolve_config_key(installed_wheel, fallback=plugin_name)
313-
enterprise_config.enable_plugin(config_key, version=version)
314+
plugin_name = enable_installed_plugin(
315+
enterprise_config, plugin_name, version, installed_wheel
316+
)
314317
enterprise_config.save()
315318

316319
ui.display_info(f"Installed {plugin_name} v{version}")
@@ -351,8 +354,9 @@ def _install_from_github_release(
351354
url, sha256, signature_mode=signature_mode
352355
)
353356

354-
config_key = resolve_config_key(installed_wheel, fallback=plugin_name)
355-
enterprise_config.enable_plugin(config_key, version=version)
357+
plugin_name = enable_installed_plugin(
358+
enterprise_config, plugin_name, version, installed_wheel
359+
)
356360
enterprise_config.save()
357361

358362
ui.display_info(f"Installed {plugin_name} v{version}")
@@ -394,8 +398,9 @@ def _install_from_github_artifact(
394398
downloader.download_from_github_artifact(url, signature_mode=signature_mode)
395399
)
396400

397-
config_key = resolve_config_key(installed_wheel, fallback=plugin_name)
398-
enterprise_config.enable_plugin(config_key, version=version)
401+
plugin_name = enable_installed_plugin(
402+
enterprise_config, plugin_name, version, installed_wheel
403+
)
399404
enterprise_config.save()
400405

401406
ui.display_info(f"Installed {plugin_name} v{version}")

ggshield/cmd/plugin/update.py

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
PluginSourceType,
2525
)
2626
from ggshield.core.plugin.downloader import DownloadError, PluginDownloader
27-
from ggshield.core.plugin.loader import PluginLoader, resolve_config_key
27+
from ggshield.core.plugin.loader import PluginLoader, enable_installed_plugin
2828
from ggshield.core.plugin.platform import get_platform_info
2929
from ggshield.core.plugin.signature import SignatureVerificationMode
3030
from ggshield.core.text_utils import pluralize
@@ -386,12 +386,6 @@ def update_cmd(
386386
signature_mode=signature_mode,
387387
bundle_bytes=bundle_bytes,
388388
)
389-
# Mirror install.py: the loader keys enablement by the
390-
# wheel's entry-point name (when present), so update
391-
# must use the same key — otherwise the row written
392-
# below falls out of sync with discover_plugins and
393-
# the plugin is silently disabled after the upgrade.
394-
config_key = resolve_config_key(wheel_path, fallback=name)
395389
installation_reports.append(
396390
_InstallationReport(
397391
name=name,
@@ -412,12 +406,21 @@ def update_cmd(
412406
_, _, wheel_path = downloader.download_from_github_release(
413407
download_url, signature_mode=signature_mode
414408
)
415-
config_key = resolve_config_key(wheel_path, fallback=name)
416409

417410
else:
418411
raise DownloadError(f"Unsupported plugin source type: {source_type}")
419412

420-
enterprise_config.enable_plugin(config_key, version=latest_version)
413+
# Mirror install.py: the loader keys enablement by the
414+
# wheel's entry-point name (when present), so update must
415+
# use the same key — otherwise the row written below falls
416+
# out of sync with discover_plugins and the plugin is
417+
# silently disabled after the upgrade. ``name`` here is
418+
# already the loader's canonical key, but a stale alias
419+
# under the wheel's distribution name from a pre-fix
420+
# install may still be on disk; ``enable_installed_plugin``
421+
# cleans that up and carries any ``auto_update: false``
422+
# forward to the canonical row.
423+
enable_installed_plugin(enterprise_config, name, latest_version, wheel_path)
421424

422425
ui.display_info(f" Updated {name} to v{latest_version}")
423426
success_count += 1

ggshield/core/plugin/downloader.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
import requests
1717

18-
from ggshield.core.dirs import get_plugins_dir
18+
from ggshield.core.dirs import get_cache_dir, get_plugins_dir
1919
from ggshield.core.plugin.client import (
2020
PluginDownloadInfo,
2121
PluginSource,
@@ -713,10 +713,23 @@ def uninstall(self, plugin_name: str) -> bool:
713713

714714
self.trust_store.revoke_plugin(plugin_dir.name)
715715
shutil.rmtree(plugin_dir)
716+
self._remove_extract_cache(plugin_dir.name)
716717

717718
logger.info("Uninstalled plugin: %s", plugin_name)
718719
return True
719720

721+
def _remove_extract_cache(self, plugin_dir_name: str) -> None:
722+
"""Best-effort removal of extracted wheel cache for an uninstalled plugin."""
723+
cache_dir = get_cache_dir() / "plugins" / plugin_dir_name
724+
try:
725+
shutil.rmtree(cache_dir)
726+
except FileNotFoundError:
727+
pass
728+
except OSError as exc:
729+
logger.debug(
730+
"Failed to remove plugin extraction cache %s: %s", cache_dir, exc
731+
)
732+
720733
def get_installed_version(self, plugin_name: str) -> Optional[str]:
721734
"""Get the installed version of a plugin (by package name or entry point name)."""
722735
manifest = self.get_manifest(plugin_name)

ggshield/core/plugin/loader.py

Lines changed: 126 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
import importlib
66
import importlib.metadata
77
import logging
8+
import os
9+
import shutil
810
import sys
911
from dataclasses import dataclass
1012
from pathlib import Path
@@ -15,7 +17,7 @@
1517

1618
from ggshield import __version__ as ggshield_version
1719
from ggshield.core.config.enterprise_config import EnterpriseConfig
18-
from ggshield.core.dirs import get_plugins_dir
20+
from ggshield.core.dirs import get_cache_dir, get_plugins_dir
1921
from ggshield.core.plugin.base import GGShieldPlugin, PluginMetadata
2022
from ggshield.core.plugin.registry import PluginRegistry
2123
from ggshield.core.plugin.signature import (
@@ -106,6 +108,57 @@ def resolve_config_key(wheel_path: Path, fallback: str) -> str:
106108
return entry_point[0]
107109

108110

111+
def enable_installed_plugin(
112+
enterprise_config: EnterpriseConfig,
113+
plugin_name: str,
114+
version: str,
115+
wheel_path: Path,
116+
) -> str:
117+
"""Enable the canonical plugin key and migrate any stale aliases.
118+
119+
Older ggshield versions wrote the enable row under the wheel's
120+
distribution name; the loader now keys discovery on the
121+
``ggshield.plugins`` entry-point name. After a fresh install or
122+
update we therefore need to:
123+
124+
1. Write the canonical row under the entry-point name (so
125+
``discover_plugins`` finds it enabled).
126+
2. Remove any pre-existing row under a known alias (the caller's
127+
``plugin_name`` — catalog reference or distribution name — and
128+
the wheel's distribution name as encoded in
129+
``wheel_path.parent.name``).
130+
3. Carry over the user's ``auto_update`` choice from any removed
131+
alias so an explicit ``auto_update: false`` survives the
132+
migration. When more than one alias exists, the strictest
133+
(``False``) wins so user intent is never silently relaxed.
134+
135+
Update.py passes the loader's discovered canonical name as
136+
``plugin_name`` (already the entry-point name), so the legacy
137+
alias only becomes visible via ``wheel_path.parent.name``. Install
138+
paths pass either the catalog reference or the distribution name,
139+
both of which become aliases here.
140+
"""
141+
config_key = resolve_config_key(wheel_path, fallback=plugin_name)
142+
143+
legacy_keys = {plugin_name, wheel_path.parent.name}
144+
legacy_keys.discard(config_key)
145+
146+
carried_auto_update: Optional[bool] = None
147+
for legacy_key in legacy_keys:
148+
legacy = enterprise_config.plugins.get(legacy_key)
149+
if legacy is None:
150+
continue
151+
# Honor the strictest setting found across aliases.
152+
if carried_auto_update is None or not legacy.auto_update:
153+
carried_auto_update = legacy.auto_update
154+
enterprise_config.remove_plugin(legacy_key)
155+
156+
enterprise_config.enable_plugin(config_key, version=version)
157+
if carried_auto_update is not None:
158+
enterprise_config.plugins[config_key].auto_update = carried_auto_update
159+
return config_key
160+
161+
109162
@dataclass
110163
class DiscoveredPlugin:
111164
"""Information about a discovered plugin."""
@@ -147,17 +200,25 @@ def discover_plugins(self) -> List[DiscoveredPlugin]:
147200
wheel_version: str = wheel_info["version"]
148201
entry_point_name: Optional[str] = wheel_info["entry_point_name"]
149202

150-
# Use entry point name as key if available, otherwise package name
203+
# Use entry point name as key if available, otherwise package name.
204+
# Older installs may have enabled the wheel distribution/package name
205+
# before ggshield learned the entry-point key. Treat that package name
206+
# as an alias so an installed+enabled plugin does not silently miss its
207+
# top-level commands.
151208
key = entry_point_name if entry_point_name else plugin_name
152209
if entry_point_name:
153210
local_entry_point_names.add(entry_point_name)
154211

212+
is_enabled = self._is_enabled(key)
213+
if key not in self.enterprise_config.plugins and plugin_name != key:
214+
is_enabled = self._is_enabled(plugin_name)
215+
155216
discovered[key] = DiscoveredPlugin(
156217
name=key,
157218
entry_point=None,
158219
wheel_path=wheel_path,
159220
is_installed=True,
160-
is_enabled=self._is_enabled(key),
221+
is_enabled=is_enabled,
161222
version=wheel_version,
162223
)
163224

@@ -272,8 +333,11 @@ def _load_from_wheel(self, wheel_path: Path) -> Optional[GGShieldPlugin]:
272333
)
273334
return None
274335

275-
# Extract wheel to a directory alongside the wheel file
276-
extract_dir = wheel_path.parent / f".{wheel_path.stem}_extracted"
336+
# Extract wheel to a per-user cache directory instead of next to the
337+
# installed wheel. This keeps loading working when a wheel is installed
338+
# in a shared/root-owned data directory but the current user can still
339+
# read it.
340+
extract_dir = self._get_extract_dir(wheel_path)
277341

278342
try:
279343
# In STRICT mode, always re-extract after verification so imports
@@ -282,15 +346,16 @@ def _load_from_wheel(self, wheel_path: Path) -> Optional[GGShieldPlugin]:
282346
not extract_dir.exists()
283347
or wheel_path.stat().st_mtime > extract_dir.stat().st_mtime
284348
):
285-
import shutil
286-
287349
if extract_dir.exists():
288350
shutil.rmtree(extract_dir)
351+
extract_dir.parent.mkdir(parents=True, exist_ok=True)
289352

290353
from ggshield.utils.archive import safe_unpack
291354

292355
safe_unpack(wheel_path, extract_dir)
293356

357+
self._prune_stale_extract_dirs(extract_dir)
358+
294359
# Add extracted directory to sys.path
295360
extract_str = str(extract_dir)
296361
if extract_str not in sys.path:
@@ -309,6 +374,60 @@ def _load_from_wheel(self, wheel_path: Path) -> Optional[GGShieldPlugin]:
309374
logger.warning("Failed to load wheel %s: %s", wheel_path, e)
310375
return None
311376

377+
def _prune_stale_extract_dirs(self, keep_dir: Path) -> None:
378+
"""Remove stale extraction dirs for the same plugin cache bucket."""
379+
parent = keep_dir.parent
380+
if not parent.exists():
381+
return
382+
383+
for path in parent.iterdir():
384+
if path == keep_dir or not path.is_dir():
385+
continue
386+
if not path.name.endswith("_extracted"):
387+
continue
388+
try:
389+
shutil.rmtree(path)
390+
except OSError as exc:
391+
logger.debug(
392+
"Failed to remove stale plugin extraction dir %s: %s", path, exc
393+
)
394+
395+
def _get_extract_cache_dir(self) -> Path:
396+
"""Return the cache dir used for extracted plugin wheels.
397+
398+
`sudo -E` on Unix can preserve a non-root HOME, which makes platformdirs
399+
point root at the invoking user's cache dir. Do not create root-owned
400+
extraction trees there: use root's own cache unless GG_CACHE_DIR was set
401+
explicitly.
402+
"""
403+
if os.environ.get("GG_CACHE_DIR"):
404+
return get_cache_dir()
405+
406+
if sys.platform != "win32" and hasattr(os, "geteuid") and os.geteuid() == 0:
407+
home = os.environ.get("HOME")
408+
try:
409+
if home and Path(home).exists() and Path(home).stat().st_uid != 0:
410+
import pwd
411+
412+
root_home = Path(pwd.getpwuid(0).pw_dir)
413+
if sys.platform == "darwin":
414+
return root_home / "Library" / "Caches" / "ggshield"
415+
return root_home / ".cache" / "ggshield"
416+
except (KeyError, OSError):
417+
pass
418+
419+
return get_cache_dir()
420+
421+
def _get_extract_dir(self, wheel_path: Path) -> Path:
422+
"""Return the per-user extraction directory for an installed wheel."""
423+
wheel_hash = compute_file_sha256(wheel_path)[:16]
424+
return (
425+
self._get_extract_cache_dir()
426+
/ "plugins"
427+
/ wheel_path.parent.name
428+
/ f"{wheel_path.stem}-{wheel_hash}_extracted"
429+
)
430+
312431
def _is_trusted_unsigned_plugin(self, wheel_path: Path) -> bool:
313432
"""Return True when the current wheel hash matches a persisted trust record."""
314433
plugin_name = wheel_path.parent.name

0 commit comments

Comments
 (0)