Skip to content
Closed
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
14 changes: 11 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,17 @@ 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, qui
# execute la base avec un jeton en ecriture + acces secrets sur une
# ref controlee par le fork) est ajoute un jour, un evenement fork est
# refuse ICI.
#
# La forme '== null' est vraie pour tout evenement sans payload de PR
# (workflow_dispatch, schedule, push), et la seconde jambe couvre
# pull_request ET pull_request_target d'un seul test. Elle ne se defait
# pas quand on ajoute un declencheur.
# Ref : issue #13874.
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
16 changes: 12 additions & 4 deletions .github/workflows/windows-self-hosted-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,17 @@ jobs:
# Acceptance B item 1 (#12704) : garde fork EXPLICITE au niveau job.
# 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, defense-in-depth qui ne se defe pas d'un clic d'UI.
# defait : si un declencheur pull_request (ou pull_request_target, qui
# execute la base avec un jeton en ecriture + acces secrets sur une
# ref controlee par le fork) est ajoute un jour, un evenement fork est
# refuse ICI, defense-in-depth qui ne se defe pas d'un clic d'UI.
#
# La forme '== null' couvre tout evenement sans payload de PR
# (workflow_dispatch, schedule, push), et la seconde jambe couvre
# pull_request ET pull_request_target d'un seul test -- cf issue #13874.
# 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 @@ -83,10 +89,12 @@ jobs:
# + restore + build Release + test xunit avec TRX logger.
name: GradeBookApp.Tests (Windows runner)
# Garde fork explicite au niveau job (cf job ci-dessus rationale partagee).
# Forme '== null' universelle (cf issue #13874) -- couvre workflow_dispatch,
# pull_request ET pull_request_target en un seul test.
# 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
11 changes: 10 additions & 1 deletion scripts/ci/check_self_hosted_runner_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,15 @@
SAME_REPO_GUARD = (
"github.event.pull_request.head.repo.full_name == github.repository"
)
# Defense-in-depth guard form that covers pull_request_target as well as
# pull_request, without enumerating the trigger. The first disjunct is true
# for every non-PR payload (workflow_dispatch, schedule, push) and the
# second covers both pull_request and pull_request_target -- the unsafe
# trigger that runs the base with write token + secrets access. Issue #13874.
SAME_REPO_GUARD_NULL_SAFE = (
"github.event.pull_request.head.repo.full_name == null "
"|| github.event.pull_request.head.repo.full_name == github.repository"
)
DEFAULT_WORKFLOWS_DIR = Path(__file__).resolve().parents[2] / ".github" / "workflows"


Expand Down Expand Up @@ -348,7 +357,7 @@ 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:
if condition not in (SAME_REPO_GUARD, SAME_REPO_GUARD_NULL_SAFE):
violations.append(Violation(
path.name,
str(job_name),
Expand Down
63 changes: 63 additions & 0 deletions scripts/tests/test_check_self_hosted_runner_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,69 @@ def test_pull_request_target_is_always_rejected(tmp_path):
assert "PULL_REQUEST_TARGET" in codes(policy.scan_workflows(tmp_path))


def test_null_safe_pull_request_guard_is_accepted(tmp_path):
"""Defense-in-depth guard form that covers pull_request_target too.

Issue #13874: the strict `head.repo.full_name == github.repository` form
leaves the job vulnerable to a future addition of `pull_request_target`
trigger (the scanner still rejects the trigger itself, but the runtime
YAML guard should not depend on enumerating events). The null-safe form
is `head.repo.full_name == null || head.repo.full_name == repo` -- true
for every non-PR payload and for any PR payload from the same repo.
"""
write_workflow(tmp_path, "pr-gate-stale-sweep", """
name: null-safe-guard
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 "SAME_REPO_GUARD" not in codes(result)
assert "PULL_REQUEST_TARGET" not in codes(result)
assert result.self_hosted_jobs == 1


def test_strict_pull_request_guard_still_accepted(tmp_path):
"""The strict canonical guard form remains valid -- no regression."""
write_workflow(tmp_path, "pr-gate-stale-sweep", """
name: strict-guard
on: [pull_request]
jobs:
test:
if: ${{ 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 "SAME_REPO_GUARD" not in codes(result)
assert result.self_hosted_jobs == 1


def test_null_safe_guard_with_extra_or_is_rejected(tmp_path):
"""A null-safe guard weakened by an extra `|| always()`-like disjunct
must still be rejected -- the new predicate accepts only the exact
null-safe form, not weakened variants.
"""
write_workflow(tmp_path, "pr-gate-stale-sweep", """
name: weak-null-safe
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 safe
""")
assert "SAME_REPO_GUARD" in codes(policy.scan_workflows(tmp_path))


def test_workflow_call_cannot_hide_self_hosted_job(tmp_path):
write_workflow(tmp_path, "callee", """
name: callee
Expand Down
Loading