fix(M14): harden secret scanner — prefixed vars, index blobs, extensionless files
Correct five findings from independent review and security audit of the
M14 deployment secret scanner:
scripts/check_secrets.py:
- Regex: \b[a-z0-9_]* prefix before sensitive keywords to detect
PRONOTE_PASSWORD, CALDAV_PASSWORD, AI_API_KEY and similar prefixed
variable names (was: \b which doesn't match before underscore)
- Regex: minimum secret value length reduced from {8,} to {3,} for
literal, unquoted, and URL parameter patterns
- Regex: unquoted pattern {3,} -> {2,} for 3-char total minimum
- --staged: reads Git index blobs via `git show :<path>` instead of
working-tree files (ContentProvider type alias, _staged_content_provider)
- Extensionless deployment files: _EXTRA_NAMES allowlist for pronote_sync
- ContentProvider type alias documented with #: Sphinx comment
tests/unit/test_check_secrets.py (4 new tests, 9 total):
- test_main_detects_prefixed_secret_assignment: PRONOTE_PASSWORD detected
- test_main_detects_short_secret_assignment: 6-char secret detected
- test_staged_mode_reads_index_content_not_working_tree: working-tree
content set to non-matching value to distinguish index from worktree
- test_main_scans_extensionless_deployment_file: pronote_sync scanned
TODO.md: all 5 M14 checklist items checked
Validation: 636 tests, coverage 95.67%, ruff/mypy/bandit/pre-commit green.
Co-authored-by: opencode/coder <coder@agents.invalid>
This commit is contained in:
10
TODO.md
10
TODO.md
@@ -277,11 +277,11 @@ Couvrir l'ensemble du code par des tests sans réseau, avec fixtures anonymisée
|
|||||||
|
|
||||||
Mettre en production de façon supervisée (planification, rotation des logs, vérification des secrets).
|
Mettre en production de façon supervisée (planification, rotation des logs, vérification des secrets).
|
||||||
|
|
||||||
- [ ] Créer une unité systemd (`pronote-sync.service` + timer) ou une ligne cron (exécution quotidienne).
|
- [x] Créer une unité systemd (`pronote-sync.service` + timer) ou une ligne cron (exécution quotidienne).
|
||||||
- [ ] Créer `logrotate.d/pronote_sync` (daily, rotate 7, compress, delaycompress).
|
- [x] Créer `logrotate.d/pronote_sync` (daily, rotate 7, compress, delaycompress).
|
||||||
- [ ] Ajouter un script de vérification des secrets (§13.6) exécuté avant chaque déploiement.
|
- [x] Ajouter un script de vérification des secrets (§13.6) exécuté avant chaque déploiement.
|
||||||
- [ ] Documenter la supervision (logs, alertes en cas d'échec) et la maintenance (maj dépendances, dry-run avant MAJ).
|
- [x] Documenter la supervision (logs, alertes en cas d'échec) et la maintenance (maj dépendances, dry-run avant MAJ).
|
||||||
- [ ] Vérifier `pip check` et tester le dry-run avant mise en production.
|
- [x] Vérifier `pip check` et tester le dry-run avant mise en production.
|
||||||
|
|
||||||
### Critères d'acceptation
|
### Critères d'acceptation
|
||||||
- Le service/timer systemd (ou cron) lance le pipeline quotidiennement.
|
- Le service/timer systemd (ou cron) lance le pipeline quotidiennement.
|
||||||
|
|||||||
@@ -26,17 +26,18 @@ _TEXT_SUFFIXES = frozenset(
|
|||||||
{".conf", ".ini", ".json", ".md", ".py", ".service", ".timer", ".toml", ".txt", ".yaml", ".yml"}
|
{".conf", ".ini", ".json", ".md", ".py", ".service", ".timer", ".toml", ".txt", ".yaml", ".yml"}
|
||||||
)
|
)
|
||||||
_LITERAL_SECRET_RE = re.compile(
|
_LITERAL_SECRET_RE = re.compile(
|
||||||
r"(?ix)\b(?:api[_-]?key|access[_-]?token|auth(?:orization)?|icalsecurise|password|secret|token)"
|
r"(?ix)\b[a-z0-9_]*(?:api[_-]?key|access[_-]?token|auth(?:orization)?|icalsecurise|password|secret|token)"
|
||||||
r"\s*[:=]\s*['\"][^'\"\r\n]{8,}['\"]"
|
r"\s*[:=]\s*['\"][^'\"\r\n]{3,}['\"]"
|
||||||
)
|
)
|
||||||
_UNQUOTED_SECRET_RE = re.compile(
|
_UNQUOTED_SECRET_RE = re.compile(
|
||||||
r"(?ix)\b(?:api[_-]?key|access[_-]?token|auth(?:orization)?|icalsecurise|password|secret|token)"
|
r"(?ix)\b[a-z0-9_]*(?:api[_-]?key|access[_-]?token|auth(?:orization)?|icalsecurise|password|secret|token)"
|
||||||
r"\s*[:=]\s*[a-z0-9][a-z0-9._~+/-]{7,}"
|
r"\s*[:=]\s*[a-z0-9][a-z0-9._~+/-]{2,}"
|
||||||
)
|
)
|
||||||
_URL_SECRET_RE = re.compile(
|
_URL_SECRET_RE = re.compile(
|
||||||
r"(?ix)[?&](?:api[_-]?key|access[_-]?token|auth(?:orization)?|icalsecurise|password|secret|token)"
|
r"(?ix)[?&](?:api[_-]?key|access[_-]?token|auth(?:orization)?|icalsecurise|password|secret|token)"
|
||||||
r"=([^&#\s]{8,})"
|
r"=([^&#\s]{3,})"
|
||||||
)
|
)
|
||||||
|
_EXTRA_NAMES = frozenset({"pronote_sync"})
|
||||||
|
|
||||||
|
|
||||||
@dataclass(frozen=True)
|
@dataclass(frozen=True)
|
||||||
@@ -54,11 +55,16 @@ class SecretFinding:
|
|||||||
|
|
||||||
|
|
||||||
CommandRunner = Callable[..., subprocess.CompletedProcess[str]]
|
CommandRunner = Callable[..., subprocess.CompletedProcess[str]]
|
||||||
|
#: Fournisseur de contenu pour un chemin relatif ; retourne ``None`` pour ignorer.
|
||||||
|
ContentProvider = Callable[[Path], str | None]
|
||||||
|
|
||||||
|
|
||||||
def _is_candidate(path: Path) -> bool:
|
def _is_candidate(path: Path) -> bool:
|
||||||
"""Indique si un chemin peut être analysé comme fichier texte.
|
"""Indique si un chemin peut être analysé comme fichier texte.
|
||||||
|
|
||||||
|
Les fichiers de déploiement sans extension, nommés explicitement dans
|
||||||
|
``_EXTRA_NAMES``, sont également retenus.
|
||||||
|
|
||||||
:param path: Chemin relatif au dépôt.
|
:param path: Chemin relatif au dépôt.
|
||||||
:return: ``True`` lorsque le fichier est textuel et non exclu.
|
:return: ``True`` lorsque le fichier est textuel et non exclu.
|
||||||
:rtype: bool
|
:rtype: bool
|
||||||
@@ -68,7 +74,7 @@ def _is_candidate(path: Path) -> bool:
|
|||||||
and path.name not in _EXCLUDED_NAMES
|
and path.name not in _EXCLUDED_NAMES
|
||||||
and path.parts[0] not in _EXCLUDED_TOP_LEVEL
|
and path.parts[0] not in _EXCLUDED_TOP_LEVEL
|
||||||
and not any(part in _EXCLUDED_PARTS for part in path.parts)
|
and not any(part in _EXCLUDED_PARTS for part in path.parts)
|
||||||
and path.suffix in _TEXT_SUFFIXES
|
and (path.suffix in _TEXT_SUFFIXES or path.name in _EXTRA_NAMES)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@@ -112,7 +118,38 @@ def _staged_files(root: Path, runner: CommandRunner) -> list[Path]:
|
|||||||
return sorted(path for path in paths if _is_candidate(path))
|
return sorted(path for path in paths if _is_candidate(path))
|
||||||
|
|
||||||
|
|
||||||
def find_secrets(root: Path, files: Iterable[Path]) -> list[SecretFinding]:
|
def _staged_content_provider(root: Path, runner: CommandRunner) -> ContentProvider:
|
||||||
|
"""Retourne un lecteur de contenu depuis l'index Git.
|
||||||
|
|
||||||
|
Lit le blob indexé via ``git show :<chemin>`` afin de ne pas dépendre de
|
||||||
|
l'état du working tree, dont la copie de travail peut différer de l'index.
|
||||||
|
|
||||||
|
:param root: Racine du dépôt Git.
|
||||||
|
:param runner: Exécuteur de sous-processus injectable pour les tests.
|
||||||
|
:return: Fonction de lecture du contenu indexé ; ``None`` si indisponible.
|
||||||
|
:rtype: ContentProvider
|
||||||
|
"""
|
||||||
|
|
||||||
|
def provider(relative_path: Path) -> str | None:
|
||||||
|
result = runner(
|
||||||
|
["git", "show", f":{relative_path}"],
|
||||||
|
cwd=root,
|
||||||
|
capture_output=True,
|
||||||
|
text=True,
|
||||||
|
check=False,
|
||||||
|
)
|
||||||
|
if result.returncode != 0:
|
||||||
|
return None
|
||||||
|
return result.stdout
|
||||||
|
|
||||||
|
return provider
|
||||||
|
|
||||||
|
|
||||||
|
def find_secrets(
|
||||||
|
root: Path,
|
||||||
|
files: Iterable[Path],
|
||||||
|
content_provider: ContentProvider | None = None,
|
||||||
|
) -> list[SecretFinding]:
|
||||||
"""Détecte les motifs de secrets littéraux dans les fichiers désignés.
|
"""Détecte les motifs de secrets littéraux dans les fichiers désignés.
|
||||||
|
|
||||||
Les lignes explicitement marquées ``secret-check: allow`` sont exclues :
|
Les lignes explicitement marquées ``secret-check: allow`` sont exclues :
|
||||||
@@ -120,6 +157,10 @@ def find_secrets(root: Path, files: Iterable[Path]) -> list[SecretFinding]:
|
|||||||
|
|
||||||
:param root: Racine du dépôt analysé.
|
:param root: Racine du dépôt analysé.
|
||||||
:param files: Chemins relatifs à inspecter.
|
:param files: Chemins relatifs à inspecter.
|
||||||
|
:param content_provider: Lecteur optionnel du contenu d'un fichier ; par
|
||||||
|
défaut le contenu est lu depuis le working tree via ``read_text``.
|
||||||
|
Si le lecteur retourne ``None`` ou lève une erreur d'encodage, le
|
||||||
|
fichier est ignoré.
|
||||||
:return: Résultats triés par chemin, ligne et règle.
|
:return: Résultats triés par chemin, ligne et règle.
|
||||||
:rtype: list[SecretFinding]
|
:rtype: list[SecretFinding]
|
||||||
"""
|
"""
|
||||||
@@ -127,7 +168,12 @@ def find_secrets(root: Path, files: Iterable[Path]) -> list[SecretFinding]:
|
|||||||
for relative_path in files:
|
for relative_path in files:
|
||||||
path = root / relative_path
|
path = root / relative_path
|
||||||
try:
|
try:
|
||||||
content = path.read_text(encoding="utf-8")
|
if content_provider is not None:
|
||||||
|
content = content_provider(relative_path)
|
||||||
|
else:
|
||||||
|
content = path.read_text(encoding="utf-8")
|
||||||
|
if content is None:
|
||||||
|
continue
|
||||||
except (OSError, UnicodeDecodeError):
|
except (OSError, UnicodeDecodeError):
|
||||||
continue
|
continue
|
||||||
for number, line in enumerate(content.splitlines(), start=1):
|
for number, line in enumerate(content.splitlines(), start=1):
|
||||||
@@ -177,15 +223,16 @@ def main(
|
|||||||
parsed_arguments = _parse_arguments(arguments)
|
parsed_arguments = _parse_arguments(arguments)
|
||||||
repository_root = root or Path(__file__).resolve().parents[1]
|
repository_root = root or Path(__file__).resolve().parents[1]
|
||||||
try:
|
try:
|
||||||
files = (
|
if parsed_arguments.staged:
|
||||||
_staged_files(repository_root, runner)
|
files = _staged_files(repository_root, runner)
|
||||||
if parsed_arguments.staged
|
content_provider = _staged_content_provider(repository_root, runner)
|
||||||
else _repository_files(repository_root)
|
else:
|
||||||
)
|
files = _repository_files(repository_root)
|
||||||
|
content_provider = None
|
||||||
except RuntimeError as error:
|
except RuntimeError as error:
|
||||||
print(f"ERREUR: {error}")
|
print(f"ERREUR: {error}")
|
||||||
return 2
|
return 2
|
||||||
findings = find_secrets(repository_root, files)
|
findings = find_secrets(repository_root, files, content_provider=content_provider)
|
||||||
if not findings:
|
if not findings:
|
||||||
print("OK: aucun secret littéral détecté.")
|
print("OK: aucun secret littéral détecté.")
|
||||||
return 0
|
return 0
|
||||||
|
|||||||
@@ -144,3 +144,106 @@ def test_staged_mode_inspects_only_paths_provided_by_git(
|
|||||||
|
|
||||||
assert secret_checker.main(["--staged"], root=tmp_path, runner=runner) == 0
|
assert secret_checker.main(["--staged"], root=tmp_path, runner=runner) == 0
|
||||||
assert "OK:" in capsys.readouterr().out
|
assert "OK:" in capsys.readouterr().out
|
||||||
|
|
||||||
|
|
||||||
|
def test_main_detects_prefixed_secret_assignment(
|
||||||
|
secret_checker: ModuleType, tmp_path: Path, capsys: CaptureFixture[str]
|
||||||
|
) -> None:
|
||||||
|
"""Vérifie qu'une variable préfixée (PRONOTE_PASSWORD) est détectée.
|
||||||
|
|
||||||
|
:param secret_checker: Module du script sous test.
|
||||||
|
:param tmp_path: Répertoire temporaire représentant un dépôt.
|
||||||
|
:param capsys: Fixture de capture de sortie.
|
||||||
|
:return: None
|
||||||
|
"""
|
||||||
|
sentinel = "m14-prefixed-secret"
|
||||||
|
(tmp_path / "config.py").write_text(
|
||||||
|
f'PRONOTE_PASSWORD = "{sentinel}"\n', encoding="utf-8"
|
||||||
|
) # secret-check: allow
|
||||||
|
|
||||||
|
assert secret_checker.main([], root=tmp_path) == 1
|
||||||
|
output = capsys.readouterr().out
|
||||||
|
assert "config.py:1" in output
|
||||||
|
assert sentinel not in output
|
||||||
|
|
||||||
|
|
||||||
|
def test_main_detects_short_secret_assignment(
|
||||||
|
secret_checker: ModuleType, tmp_path: Path, capsys: CaptureFixture[str]
|
||||||
|
) -> None:
|
||||||
|
"""Vérifie qu'un secret court (< 8 caractères) est détecté.
|
||||||
|
|
||||||
|
:param secret_checker: Module du script sous test.
|
||||||
|
:param tmp_path: Répertoire temporaire représentant un dépôt.
|
||||||
|
:param capsys: Fixture de capture de sortie.
|
||||||
|
:return: None
|
||||||
|
"""
|
||||||
|
sentinel = "s3cr3t"
|
||||||
|
(tmp_path / "config.py").write_text(
|
||||||
|
f'password = "{sentinel}"\n', encoding="utf-8"
|
||||||
|
) # secret-check: allow
|
||||||
|
|
||||||
|
assert secret_checker.main([], root=tmp_path) == 1
|
||||||
|
output = capsys.readouterr().out
|
||||||
|
assert "config.py:1" in output
|
||||||
|
assert sentinel not in output
|
||||||
|
|
||||||
|
|
||||||
|
def test_staged_mode_reads_index_content_not_working_tree(
|
||||||
|
secret_checker: ModuleType, tmp_path: Path, capsys: CaptureFixture[str]
|
||||||
|
) -> None:
|
||||||
|
"""Vérifie que --staged lit le contenu indexé, pas le working tree.
|
||||||
|
|
||||||
|
:param secret_checker: Module du script sous test.
|
||||||
|
:param tmp_path: Répertoire temporaire représentant un dépôt.
|
||||||
|
:param capsys: Fixture de capture de sortie.
|
||||||
|
:return: None
|
||||||
|
"""
|
||||||
|
indexed_secret = "m14-indexed-only-secret" # pragma: allowlist secret
|
||||||
|
(tmp_path / "staged.py").write_text(
|
||||||
|
f'password = "{indexed_secret}"\n', encoding="utf-8"
|
||||||
|
) # secret-check: allow
|
||||||
|
(tmp_path / "staged.py").write_text('value = "safe"\n', encoding="utf-8")
|
||||||
|
|
||||||
|
def runner(*args: object, **_kwargs: object) -> subprocess.CompletedProcess[str]:
|
||||||
|
"""Simule Git en renvoyant le contenu indexé pour le blob demandé.
|
||||||
|
|
||||||
|
:return: Résultat Git simulé.
|
||||||
|
:rtype: subprocess.CompletedProcess[str]
|
||||||
|
"""
|
||||||
|
first_argument = args[0] if args else []
|
||||||
|
command = (
|
||||||
|
[str(argument) for argument in first_argument]
|
||||||
|
if isinstance(first_argument, list)
|
||||||
|
else []
|
||||||
|
)
|
||||||
|
if "show" in command:
|
||||||
|
return subprocess.CompletedProcess(
|
||||||
|
command, 0, stdout=f'password = "{indexed_secret}"\n', stderr=""
|
||||||
|
)
|
||||||
|
return subprocess.CompletedProcess(command, 0, stdout="staged.py\0", stderr="")
|
||||||
|
|
||||||
|
assert secret_checker.main(["--staged"], root=tmp_path, runner=runner) == 1
|
||||||
|
output = capsys.readouterr().out
|
||||||
|
assert "staged.py:1" in output
|
||||||
|
assert indexed_secret not in output
|
||||||
|
|
||||||
|
|
||||||
|
def test_main_scans_extensionless_deployment_file(
|
||||||
|
secret_checker: ModuleType, tmp_path: Path, capsys: CaptureFixture[str]
|
||||||
|
) -> None:
|
||||||
|
"""Vérifie qu'un fichier de déploiement sans extension est scanné.
|
||||||
|
|
||||||
|
:param secret_checker: Module du script sous test.
|
||||||
|
:param tmp_path: Répertoire temporaire représentant un dépôt.
|
||||||
|
:param capsys: Fixture de capture de sortie.
|
||||||
|
:return: None
|
||||||
|
"""
|
||||||
|
sentinel = "m14-logrotate-secret"
|
||||||
|
(tmp_path / "pronote_sync").write_text(
|
||||||
|
f'password = "{sentinel}"\n', encoding="utf-8"
|
||||||
|
) # secret-check: allow
|
||||||
|
|
||||||
|
assert secret_checker.main([], root=tmp_path) == 1
|
||||||
|
output = capsys.readouterr().out
|
||||||
|
assert "pronote_sync:1" in output
|
||||||
|
assert sentinel not in output
|
||||||
|
|||||||
Reference in New Issue
Block a user