Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions .github/workflows/linux-self-hosted-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,18 @@ jobs:
# Garde fork EXPLICITE au niveau job (defense-in-depth, pattern maison).
# Le declencheur actuel est workflow_dispatch seul -- un fork ne peut pas
# l'emettre -- mais "pas de trigger" est un etat qu'une ligne de YAML
# defait : si un declencheur pull_request est ajoute un jour, un evenement
# fork est refuse ICI.
if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository
# defait : si un declencheur pull_request (ou pull_request_target) est
# ajoute un jour, un evenement fork est refuse ICI.
#
# Forme universelle (#13874) : la garde ne s'appuie plus sur la valeur
# textuelle de github.event_name (qui distingue pull_request de
# pull_request_target -- le second est le vecteur d'exfiltration classique
# sur runner self-hosted avec token en ecriture + acces secrets). Elle
# teste la presence du champ pull_request (null hors PR) + l'identite du
# repo source en un seul predicat, couvrant les deux variantes de
# declencheur d'un coup. Un fork (head.repo.full_name != repository) est
# refuse que le trigger s'appelle pull_request ou pull_request_target.
if: github.event.pull_request.head.repo.full_name == null || github.event.pull_request.head.repo.full_name == github.repository
runs-on: [self-hosted, coursia-ephemeral, coursia-linux]
timeout-minutes: 10
concurrency:
Expand Down
9 changes: 8 additions & 1 deletion .github/workflows/orphaned-delivery-scan.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,14 @@ jobs:
# On [closed], restrict to same-repo PRs: fork PRs cannot create an
# internal base != main orphan and their limited token/fetch access would
# only produce spurious failures on an advisory job.
if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository
#
# Forme universelle (#13874) : teste la presence du champ pull_request
# (null pour schedule et workflow_dispatch, ce qui couvre ces triggers
# legitimes) puis l'identite du repo source en un seul predicat. Couvre
# pull_request ET pull_request_target d'un coup (le second est le vecteur
# d'exfiltration classique sur runner public -- voir le commentaire
# equivalent dans linux-self-hosted-tests.yml).
if: github.event.pull_request.head.repo.full_name == null || github.event.pull_request.head.repo.full_name == github.repository
steps:
- name: Checkout (full history, blobless)
# Partial clone, serie #11843 (workflow 2). The scan interrogates the
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/windows-self-hosted-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ jobs:
# fork est refuse ICI, defense-in-depth qui ne se defe pas d'un clic d'UI.
# Garde selection : inputs.target == 'confinement' (sinon ce job n'est
# pas eligible -- un dispatch `target=dotnet` ne planifie que le job .NET).
if: (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) && inputs.target == 'confinement'
if: (github.event.pull_request.head.repo.full_name == null || github.event.pull_request.head.repo.full_name == github.repository) && inputs.target == 'confinement'
runs-on: [self-hosted, coursia-ephemeral, coursia-fast-guards]
timeout-minutes: 15
concurrency:
Expand Down Expand Up @@ -86,7 +86,7 @@ jobs:
# Garde selection : inputs.target == 'dotnet' (sinon ce job n'est pas
# eligible -- un dispatch `target=confinement` ne planifie que le job
# confinement).
if: (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) && inputs.target == 'dotnet'
if: (github.event.pull_request.head.repo.full_name == null || github.event.pull_request.head.repo.full_name == github.repository) && inputs.target == 'dotnet'
runs-on: [self-hosted, coursia-ephemeral, coursia-fast-guards]
timeout-minutes: 20
# Groupe separe : un dispatch .NET ne doit PAS annuler un dispatch
Expand Down
81 changes: 79 additions & 2 deletions scripts/ci/check_self_hosted_runner_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,17 @@
SAME_REPO_GUARD = (
"github.event.pull_request.head.repo.full_name == github.repository"
)
# Universal form (#13874): ne s'appuie pas sur la valeur textuelle de
# github.event_name (qui distingue pull_request de pull_request_target,
# le second etant le vecteur d'exfiltration classique sur runner public).
# Teste la presence du champ pull_request + l'identite du repo source,
# couvrant les deux variantes de declencheur en un seul predicat.
# Acceptee en plus de la forme directe SAME_REPO_GUARD.
SAME_REPO_GUARD_UNIVERSAL = (
"github.event.pull_request.head.repo.full_name == null "
"|| github.event.pull_request.head.repo.full_name == github.repository"
)
ACCEPTED_SAME_REPO_GUARDS = frozenset({SAME_REPO_GUARD, SAME_REPO_GUARD_UNIVERSAL})
DEFAULT_WORKFLOWS_DIR = Path(__file__).resolve().parents[2] / ".github" / "workflows"


Expand Down Expand Up @@ -204,6 +215,68 @@ def _normalise_condition(value: Any) -> str:
return " ".join(condition.split())


def _starts_with_accepted_guard(condition: str) -> bool:
"""True iff ``condition`` leads with one of the accepted same-repo
guards, optionally wrapped in parens and optionally followed by `&& ...`
selection predicates. The guard must be the LEAD predicate, and any
suffix joined by `||` is rejected (the `||` would let the guard fall
through). The standalone tests ``test_guard_weakened_by_or_is_rejected``
and ``test_universal_guard_weakened_by_or_is_rejected`` verify that
`|| always()` is refused.
"""
if not condition:
return False
# Short-circuit: the whole condition is itself an accepted guard.
if condition in ACCEPTED_SAME_REPO_GUARDS:
return True
lead = condition
suffix = ""
if lead.startswith("("):
depth = 0
for idx, ch in enumerate(lead):
if ch == "(":
depth += 1
elif ch == ")":
depth -= 1
if depth == 0:
lead = lead[: idx + 1]
suffix = condition[idx + 1 :].lstrip()
break
else:
# Bare form: scan character by character for a top-level ` && `
# or ` || ` (4 chars including surrounding spaces). An external
# `&&` joining a selection predicate is OK; an external `||` is
# rejected because it lets the guard fall through. The internal
# `||` of the universal form is captured by the short-circuit
# above (whole-condition match), so this branch only fires when
# the condition is followed by an external operator.
depth = 0
i = 0
while i < len(lead) - 3:
ch = lead[i]
if ch == "(":
depth += 1
i += 1
continue
if ch == ")":
depth -= 1
i += 1
continue
if depth == 0 and lead[i : i + 4] in (" && ", " || "):
lead = lead[:i]
suffix = condition[i:]
break
i += 1
for accepted in ACCEPTED_SAME_REPO_GUARDS:
if lead == accepted or lead == "(" + accepted + ")":
if not suffix:
return True
if suffix.lstrip().startswith("&&"):
return True
return False
return False


def scan_workflows(workflows_dir: Path = DEFAULT_WORKFLOWS_DIR) -> ScanResult:
yaml = _load_yaml()
if yaml is None:
Expand Down Expand Up @@ -348,12 +421,16 @@ def scan_workflows(workflows_dir: Path = DEFAULT_WORKFLOWS_DIR) -> ScanResult:

if "pull_request" in triggers:
condition = _normalise_condition(job.get("if"))
if condition != SAME_REPO_GUARD:
# Accept either the bare guard or the guard followed by a
# selection predicate (e.g. `&& inputs.target == '...'`)
# joined with `&&` -- the guard must be the lead predicate,
# never weakened by `||` (cf. test_guard_weakened_by_or_is_rejected).
if not _starts_with_accepted_guard(condition):
violations.append(Violation(
path.name,
str(job_name),
"SAME_REPO_GUARD",
"pull_request self-hosted job must use the exact same-repo job guard",
"pull_request self-hosted job must lead with the same-repo job guard",
))

return ScanResult(len(paths), jobs_scanned, self_hosted_jobs, violations, broken)
Expand Down
89 changes: 89 additions & 0 deletions scripts/tests/test_check_self_hosted_runner_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,95 @@ def test_safe_pull_request_job_is_accepted(tmp_path):
assert result.self_hosted_jobs == 1


def test_universal_pull_request_guard_is_accepted(tmp_path):
"""#13874 : la forme universelle (test du repo source sans enumerer
github.event_name) couvre pull_request ET pull_request_target en un seul
predicat. Elle doit etre acceptee par le checker au meme titre que la
forme directe."""
write_workflow(tmp_path, "pr-gate-stale-sweep", """
name: safe-universal
on: [pull_request]
jobs:
test:
if: ${{ github.event.pull_request.head.repo.full_name == null || github.event.pull_request.head.repo.full_name == github.repository }}
runs-on: [self-hosted, coursia-ephemeral, coursia-fast-guards]
steps:
- run: echo safe
""")
result = policy.scan_workflows(tmp_path)
assert result.broken == []
assert result.violations == []
assert result.self_hosted_jobs == 1


def test_universal_guard_with_combined_target_is_accepted(tmp_path):
"""#13874 : variante parentee -- la garde universelle combinee avec
une selection de job (inputs.target == '...') reste acceptee. Cas reel :
windows-self-hosted-tests.yml factorise la garde avec un `&&` de
selection, le checker ne doit pas confondre ce `&&` avec une
fragilisation. Trigger pull_request pour exercer le SAME_REPO_GUARD
(le test serait silently-skipped sur workflow_dispatch seul)."""
write_workflow(tmp_path, "windows-self-hosted-tests", """
name: windows-universal
on: [pull_request]
jobs:
test:
if: ${{ (github.event.pull_request.head.repo.full_name == null || github.event.pull_request.head.repo.full_name == github.repository) && inputs.target == 'confinement' }}
runs-on: [self-hosted, coursia-ephemeral, coursia-fast-guards]
steps:
- run: echo safe
""")
result = policy.scan_workflows(tmp_path)
assert result.broken == []
assert result.violations == []
assert result.self_hosted_jobs == 1


def test_universal_guard_weakened_by_or_is_rejected(tmp_path):
"""#13874 FN-safety : meme avec la forme universelle, l'ajout d'un
`|| always()` ou `|| true` reintroduit le trou. La garde universelle
doit etre le seul predicat -- le checker refuse tout court-circuit."""
write_workflow(tmp_path, "pr-gate-stale-sweep", """
name: weakened
on: [pull_request]
jobs:
test:
if: ${{ (github.event.pull_request.head.repo.full_name == null || github.event.pull_request.head.repo.full_name == github.repository) || always() }}
runs-on: [self-hosted, coursia-ephemeral, coursia-fast-guards]
steps:
- run: echo unsafe
""")
assert "SAME_REPO_GUARD" in codes(policy.scan_workflows(tmp_path))


def test_event_name_or_pattern_is_rejected(tmp_path):
"""#13874 FN-safety : la forme `github.event_name != 'pull_request' ||`
(l'ancienne garde faible qui laisse passer pull_request_target) doit
toujours etre refusee par le checker -- c'est precisement le defaut
que l'issue signale. Le test pose les DEUX triggers pour reproduire
le scenario d'evolution : quelqu'un ajoute pull_request_target a un
workflow qui avait deja pull_request."""
write_workflow(tmp_path, "pr-gate-stale-sweep", """
name: weak-old
on:
pull_request:
pull_request_target:
jobs:
test:
if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }}
runs-on: [self-hosted, coursia-ephemeral, coursia-fast-guards]
steps:
- run: echo unsafe
""")
codes_set = codes(policy.scan_workflows(tmp_path))
# La policy doit signaler le PULL_REQUEST_TARGET (deja couvert) ET le
# SAME_REPO_GUARD (la forme ne correspond ni a la garde directe ni a
# la forme universelle, elle est dans la zone grise entre les deux et
# doit etre refusee).
assert "PULL_REQUEST_TARGET" in codes_set
assert "SAME_REPO_GUARD" in codes_set


def test_allowed_workflow_dispatch_job_does_not_need_pr_guard(tmp_path):
write_workflow(tmp_path, "pr-gate-stale-sweep", """
name: manual
Expand Down
Loading