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
1 change: 1 addition & 0 deletions .github/cspell.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
version: "0.2"
import:
- ../cspell.yaml
Expand All @@ -5,6 +5,7 @@
- CODEOWNERS*
- arm-leases/**
- policies/**
- protected-labels.yml
- workflows/*.lock.yml
- workflows/**/*.md
words:
Expand Down
34 changes: 0 additions & 34 deletions .github/namespace-approvers.yml

This file was deleted.

64 changes: 64 additions & 0 deletions .github/protected-labels.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# Protected Labels Configuration
# This file defines which labels can only be applied by authorized users.
# If an unauthorized user applies a protected label, the bot removes it.
#
# The problem: service teams self-applying "Approved" labels to bypass architect review.
# This workflow ensures only authorized reviewers can sign off via labels.
#
# Format:
# global-approvers: users who can apply ANY protected label
# LabelName: users who can apply THIS specific label (in addition to global)
#
# To update: submit a PR modifying this file.

# Global approvers can apply any protected label.
global-approvers:
- lmazuel
- samvaity
- praveenkuttappan

# Namespace approval labels (plane-aware)
namespace-dotnet-approved:
management-plane:
- ArthurMa1978
- m-nash
data-plane:
- jsquire
- m-nash
namespace-java-approved:
management-plane:
- ArthurMa1978
- m-nash
data-plane:
- JonathanGiles
- alzimmermsft
namespace-python-approved:
management-plane:
- ArthurMa1978
- m-nash
data-plane:
- xirzec
- kashifkhan
namespace-typescript-approved:
management-plane:
- ArthurMa1978
- m-nash
data-plane:
- xirzec
- maorleger
namespace-go-approved:
management-plane:
- ArthurMa1978
- m-nash
data-plane:
- jhendrixMSFT
namespace-rust-approved:
management-plane:
- ArthurMa1978
- m-nash
data-plane:
- heaths
namespace-approved-all:
management-plane:
- ArthurMa1978
- m-nash
34 changes: 34 additions & 0 deletions .github/workflows/protected-labels.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Protected Labels - Enforces that only authorized users can apply certain labels.
# Reads authorized users from .github/protected-labels.yml.
# If an unauthorized user applies a protected label, the bot removes it and posts a warning.
name: "Protected Labels"

on:
pull_request_target:
types:
- labeled

permissions:
contents: read
pull-requests: write

jobs:
check-label:
name: Check Protected Label
runs-on: ubuntu-24.04
steps:
- uses: actions/checkout@v6
with:
sparse-checkout: |
.github/

- name: Install dependencies for github-script actions
uses: ./.github/actions/install-deps-github-script

- name: Validate label authorization
uses: actions/github-script@v8
with:
script: |
const { default: checkLabel } =
await import('${{ github.workspace }}/.github/workflows/src/protected-labels/check-label.js');
await checkLabel({ github, context, core });
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
# Namespace Approval Process

## Overview
Expand Down Expand Up @@ -63,7 +63,7 @@

## Authorized Approvers

Defined in [`.github/namespace-approvers.yml`](../../../namespace-approvers.yml):
Defined in [`.github/protected-labels.yml`](../../../protected-labels.yml) (namespace entries use plane-aware format):

<!-- cspell:disable -->

Expand Down Expand Up @@ -97,7 +97,7 @@

## Configuration

To update approvers, submit a PR modifying `.github/namespace-approvers.yml`. Changes take effect immediately on merge.
To update approvers, submit a PR modifying `.github/protected-labels.yml`. Changes take effect immediately on merge.

## Format Validation

Expand All @@ -112,7 +112,7 @@
- **No fork code execution:** The workflow only reads configuration files (approvers YAML) from the base branch, never from the PR branch.
- **Unauthorized label reversal:** If an unauthorized user adds an approval label, the workflow removes it and posts a warning comment.
- **Unauthorized removal guard:** If an unauthorized user removes a pending label, the workflow re-applies it.
- **Approver allowlist:** Only users listed in `.github/namespace-approvers.yml` can approve namespaces.
- **Approver allowlist:** Only users listed in `.github/protected-labels.yml` can approve namespaces.

## Relationship to API Review

Expand Down
79 changes: 75 additions & 4 deletions .github/workflows/src/namespace-approval/approvers.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import { readFile } from "fs/promises";
import yaml from "js-yaml";

Expand All @@ -7,15 +7,86 @@
* @property {{ all?: string[] }} [management-plane]
*/

const APPROVERS_PATH = ".github/namespace-approvers.yml";
const PROTECTED_LABELS_PATH = ".github/protected-labels.yml";

/**
* Load the approvers configuration from the YAML file.
* Derive namespace ApproversConfig from protected-labels.yml.
*
* Reads plane-aware namespace entries and builds the nested structure
* that validate-approval.js expects:
* data-plane: { dotnet: [...], java: [...], global: [...] }
* management-plane: { all: [...] }
*
* @param {string} [path]
* @returns {Promise<ApproversConfig>}
*/
export async function loadApproversConfig(path = APPROVERS_PATH) {
export async function loadApproversConfig(path = PROTECTED_LABELS_PATH) {
const content = await readFile(path, "utf8");
return /** @type {ApproversConfig} */ (yaml.load(content));
const config = /** @type {Record<string, unknown>} */ (yaml.load(content));

/** @type {Record<string, string[]>} */
const dataPlane = {};
/** @type {string[]} */
let mgmtAll = [];

// Include global-approvers in data-plane.global so getAllApprovers()
// (used by handleUnlabeled) recognizes them as authorized.
const globalApprovers = /** @type {string[]} */ (
/** @type {unknown} */ (config["global-approvers"] ?? [])
);

for (const [label, entry] of Object.entries(config)) {
if (label === "global-approvers") continue;

// Match namespace-<lang>-approved or namespace-approved-all
let lang;
if (label === "namespace-approved-all") {
lang = "all";
} else {
const match = label.match(/^namespace-(\w+)-approved$/);
if (!match) continue;
lang = match[1];
}

// Flat entry (backward compat) - treat all users as data-plane
if (Array.isArray(entry)) {
const users = /** @type {string[]} */ (/** @type {unknown} */ (entry));
if (lang === "all") {
dataPlane.global = users;
} else {
dataPlane[lang] = users;
}
continue;
}

// Plane-aware entry
if (entry && typeof entry === "object") {
const planeEntry = /** @type {{ "management-plane"?: string[], "data-plane"?: string[] }} */ (
entry
);
if (planeEntry["management-plane"]) {
// Collect unique mgmt approvers across all namespace labels
mgmtAll = [...new Set([...mgmtAll, ...planeEntry["management-plane"]])];
}
if (planeEntry["data-plane"]) {
if (lang === "all") {
dataPlane.global = planeEntry["data-plane"];
} else {
dataPlane[lang] = planeEntry["data-plane"];
}
}
}
}

// Merge global-approvers into data-plane.global
if (globalApprovers.length > 0) {
dataPlane.global = [...new Set([...(dataPlane.global ?? []), ...globalApprovers])];
}

return {
"data-plane": dataPlane,
// Intentionally unions all mgmt approvers into one list - any mgmt approver
// for any language can approve any other language on mgmt plane.
"management-plane": { all: mgmtAll },
};
}
4 changes: 2 additions & 2 deletions .github/workflows/src/namespace-approval/post-results.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import { unlink, writeFile } from "fs/promises";
import { join } from "path";
import { z } from "zod";
Expand Down Expand Up @@ -95,7 +95,7 @@
const approvers = approversConfig["data-plane"]?.[language];
if (!approvers) {
throw new Error(
`No approvers configured for language "${language}" in .github/namespace-approvers.yml`,
`No approvers configured for language "${language}" in .github/protected-labels.yml`,
);
}
return approvers;
Expand Down Expand Up @@ -205,7 +205,7 @@
if (resetLanguages && resetLanguages.length > 0) {
body += `\n> ⚠️ **Namespace changed** — approvals for ${resetLanguages.join(", ")} have been reset.\n`;
}
body += `\n_Approver list: [.github/namespace-approvers.yml](../blob/${baseRef}/.github/namespace-approvers.yml)_\n`;
body += `\n_Approver list: [.github/protected-labels.yml](../blob/${baseRef}/.github/protected-labels.yml)_\n`;
body += `_Process: [.github/workflows/src/namespace-approval/NAMESPACE-REVIEW-PROCESS.md](../blob/${baseRef}/.github/workflows/src/namespace-approval/NAMESPACE-REVIEW-PROCESS.md)_\n`;
body += `_Namespaces extracted from tspconfig.yaml emitter options_`;
return body;
Expand Down
46 changes: 12 additions & 34 deletions .github/workflows/src/namespace-approval/validate-approval.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import { extractInputs } from "../context.js";
import { loadApproversConfig } from "./approvers.js";
import { removeLabelIfPresent } from "./labels.js";
Expand Down Expand Up @@ -61,7 +61,8 @@

/** @type {string[]} */
const allApprovers = getAllApprovers(approversConfig);
if (allApprovers.includes(actor)) {
const actorLower = actor.toLowerCase();
if (allApprovers.some((u) => u.toLowerCase() === actorLower)) {
core.info(`${actor} is an authorized approver, allowing label removal`);
return;
}
Expand Down Expand Up @@ -91,7 +92,6 @@
async function handleLabeled({
github,
core,
approversConfig,
owner,
repo,
targetLabel,
Expand All @@ -104,10 +104,10 @@
let langsToApprove;

if (targetLabel === "namespace-approved-all") {
/** @type {string[]} */
const allApprovers = getAllApprovers(approversConfig);

if (!allApprovers.includes(actor)) {
// namespace-approved-all is a shortcut for management-plane only.
// On mgmt PRs, a single label approves all pending languages at once.
// Auth (who can apply) is enforced by check-label.js (protected-labels workflow).
if (!isMgmt) {
await github.rest.issues.removeLabel({
owner,
repo,
Expand All @@ -118,7 +118,7 @@
owner,
repo,
issue_number: prNumber,
body: `⚠️ @${actor} is not authorized to use \`namespace-approved-all\`. This shortcut is available to authorized namespace approvers.\n\nLabel removed.`,
body: `⚠️ \`namespace-approved-all\` is only available on management-plane PRs. Use per-language \`namespace-<lang>-approved\` labels instead.\n\nLabel removed.`,
});
return;
}
Expand All @@ -133,33 +133,11 @@
return;
}

// Auth (who can apply) is enforced by check-label.js (protected-labels workflow).
// Both workflows run concurrently on labeled events, so there's a brief window
// where this processes before check-label.js removes an unauthorized label.
// State reconciles on the next event (label removal triggers unlabeled handler).
const lang = match[1];
/** @type {string[]} */
let authorizedList;
if (isMgmt && approversConfig["management-plane"]?.all) {
authorizedList = approversConfig["management-plane"].all;
} else {
const langApprovers = approversConfig["data-plane"]?.[lang] ?? [];
const globalApprovers = approversConfig["data-plane"]?.global ?? [];
authorizedList = [...new Set([...langApprovers, ...globalApprovers])];
}

if (!authorizedList.includes(actor)) {
await github.rest.issues.removeLabel({
owner,
repo,
issue_number: prNumber,
name: targetLabel,
});
await github.rest.issues.createComment({
owner,
repo,
issue_number: prNumber,
body: `⚠️ @${actor} is not authorized to approve **${lang}** namespace. Authorized approvers: ${authorizedList.join(", ")}.\n\nLabel \`${targetLabel}\` has been removed.`,
});
return;
}

langsToApprove = [lang];
}

Expand Down Expand Up @@ -268,7 +246,7 @@
);
const targetLabel = payload.label.name;
const actor = payload.sender.login;
const isMgmt = labels.includes("Mgmt");
const isMgmt = labels.includes("Mgmt") || labels.includes("resource-manager");

if (payload.action === "unlabeled") {
return await handleUnlabeled({
Expand Down
Loading