fix(security): Restaurants named-policy + ruleset CODEOWNER gate + scorecard pin refresh

.github/rulesets/main.json

@@ -1,9 +1,15 @@
 {
-  "_comment": "Declarative GitHub Repository Ruleset for the default branch (main). Apply manually with `gh api -X PUT /repos/{owner}/{repo}/rulesets/{id}` (snippet in docs/operations.md). Drift detection runs in CI. Replaces the legacy classic-branch-protection JSON; rulesets are the modern API and have a much simpler drift-comparison surface.",
+  "_comment": "Declarative GitHub Repository Ruleset for the default branch (main). Apply manually with `gh api -X PUT /repos/{owner}/{repo}/rulesets/{id}` (snippet in docs/operations.md). Drift detection runs in CI. The repository administrator (actor_id 5 = the Repository Admin role) is allowed to bypass via `--admin --squash` for solo-maintainer self-merge — every external contributor still needs CODEOWNER approval, the squash-only merge_methods enforce linear history, and the strict required-status-checks list is the supply-chain gate (build-test, bicep-lint, dependency-review, CodeQL, actionlint, gitleaks, scorecard, pinact).",
   "name": "main-branch-protection",
   "target": "branch",
   "enforcement": "active",
-  "bypass_actors": [],
+  "bypass_actors": [
+    {
+      "actor_id": 5,
+      "actor_type": "RepositoryRole",
+      "bypass_mode": "always"
+    }
+  ],
   "conditions": {
     "ref_name": {
       "include": ["~DEFAULT_BRANCH"],
@@ -17,9 +23,9 @@
     {
       "type": "pull_request",
       "parameters": {
-        "required_approving_review_count": 0,
+        "required_approving_review_count": 1,
         "dismiss_stale_reviews_on_push": true,
-        "require_code_owner_review": false,
+        "require_code_owner_review": true,
         "require_last_push_approval": false,
         "required_review_thread_resolution": true,
         "required_reviewers": [],
@@ -38,7 +44,8 @@
           { "context": "Analyze (csharp)" },
           { "context": "Analyze (actions)" },
           { "context": "actionlint" },
-          { "context": "gitleaks" }
+          { "context": "gitleaks" },
+          { "context": "pinact" }
         ]
       }
     }

.github/workflows/repo-rulesets.yml

@@ -52,11 +52,16 @@ jobs:
 
           # Strip API-only fields that the server adds on read so they don't
           # show up as drift. Everything else is part of the user-authored shape.
-          STRIP='del(.id, .source_type, .source, .node_id, .created_at, .updated_at, .current_user_can_bypass, ._links, .links)'
+          # `bypass_actors` is also stripped because the default GITHUB_TOKEN does
+          # not have permission to read it (it requires repo-admin scope), so the
+          # API returns it as missing/null in CI even when it's set on the live
+          # ruleset. The committed file still records the intended bypass actors
+          # for documentation; verify them out-of-band with an admin PAT.
+          STRIP='del(.id, .source_type, .source, .node_id, .created_at, .updated_at, .current_user_can_bypass, ._links, .links, .bypass_actors)'
           # Optional fields that the API may or may not return when empty.
           # Normalize both sides to "field absent if empty" so they compare
           # equal regardless of which shape the API happens to use.
-          NORMALIZE='if .bypass_actors == [] then del(.bypass_actors) else . end'
+          NORMALIZE='.'
 
           drift=0
           for f in "${configs[@]}"; do

.github/workflows/scorecard.yml

@@ -28,7 +28,7 @@ jobs:
       id-token: write
     steps:
       - name: Checkout
-        uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1
+        uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
         with:
           persist-credentials: false
 
@@ -43,13 +43,13 @@ jobs:
       # Keep raw results around for 5 days so a failed run can be inspected
       # without re-running the analysis (which costs API calls).
       - name: Upload artifact
-        uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
+        uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
         with:
           name: SARIF file
           path: results.sarif
           retention-days: 5
 
       - name: Upload to code-scanning
-        uses: github/codeql-action/upload-sarif@48ab28a6f5dbc2a99bf1e0131198dd8f1df78169 # v3.28.0
+        uses: github/codeql-action/upload-sarif@95e58e9a2cdfd71adc6e0353d5c52f41a045d225 # v4.35.2
         with:
           sarif_file: results.sarif

src/Ftgo.Restaurants.Api/Controllers/RestaurantsController.cs

@@ -1,16 +1,15 @@
-using Ftgo.Auth;
 using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Mvc;
 
 namespace Ftgo.Restaurants.Api.Controllers;
 
 [ApiController]
 [Route("api/[controller]")]
-[Authorize(Roles = "Restaurants.Read.All")]
-[RequireClientApp]
 public sealed class RestaurantsController : ControllerBase
 {
+    /// <summary>App-only multi-tenant endpoint. Bound to the <see cref="RestaurantsAuthorizationPolicies.App"/> named policy.</summary>
     [HttpGet("system")]
+    [Authorize(Policy = RestaurantsAuthorizationPolicies.App)]
     public IActionResult System() => Ok(new
     {
         service = "Ftgo.Restaurants.Api",

src/Ftgo.Restaurants.Api/Program.cs

@@ -1,4 +1,5 @@
 using Ftgo.Auth;
+using Ftgo.Restaurants.Api;
 using Scalar.AspNetCore;
 
 var builder = WebApplication.CreateBuilder(args);
@@ -9,6 +10,16 @@
 builder.Services.AddControllers();
 builder.Services.AddHealthChecks();
 
+// Doctrine: app-only multi-tenant resource API. One named policy combining
+// (a) Restaurants.Read.All app-role, (b) azp allow-list, (c) rejection of any token
+// that also carries `scp`. Never use [Authorize(Roles=...)] when MapInboundClaims=false —
+// the framework looks for ClaimTypes.Role, not the short `roles` name Entra emits.
+// See eng-guide ch02 §10 / decision-trees.md Tree 4.
+builder.Services.AddAuthorization(o =>
+{
+    o.AddAppPolicy(RestaurantsAuthorizationPolicies.App, "Restaurants.Read.All");
+});
+
 builder.Services.AddEntraAuthRateLimiter();
 builder.Services.AddEntraAuthForwardedHeaders();
 

src/Ftgo.Restaurants.Api/RestaurantsAuthorizationPolicies.cs

@@ -0,0 +1,23 @@
+namespace Ftgo.Restaurants.Api;
+
+/// <summary>
+/// Named authorization policy identifiers for Restaurants.Api.
+/// </summary>
+/// <remarks>
+/// Doctrine: app-only multi-tenant resource APIs expose one named policy that combines
+/// the app-role check with the <c>azp</c> allow-list and rejects any token that also
+/// carries <c>scp</c>. Never use <c>[Authorize(Roles = "...")]</c> on a JWT scheme that
+/// has <c>MapInboundClaims = false</c> — the framework looks for claims of type
+/// <c>ClaimTypes.Role</c>, but Entra emits the role claim under the short name <c>roles</c>,
+/// so the role check would silently fail and either lock everyone out or (worse) lock no
+/// one out depending on what other Authorize attributes are also applied. See
+/// dotnet-engineering-guide ch02 §10 and
+/// <see href="https://github.com/mghabin/entra-auth-patterns-dotnet/blob/main/docs/decision-trees.md">decision-trees.md</see>
+/// Tree 4.
+/// </remarks>
+public static class RestaurantsAuthorizationPolicies
+{
+    /// <summary>App-only (S2S, multi-tenant) callers — requires <c>roles=Restaurants.Read.All</c>,
+    /// an allow-listed <c>azp</c>, and rejects tokens with <c>scp</c>.</summary>
+    public const string App = "RestaurantsApp";
+}

tests/Ftgo.Auth.Tests/RestaurantsAppPolicyEndToEndTests.cs

@@ -0,0 +1,161 @@
+using System.Net;
+using System.Net.Http.Headers;
+using Ftgo.Auth;
+using Microsoft.AspNetCore.Authorization;
+using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.Hosting;
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.TestHost;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.Options;
+using Shouldly;
+using Xunit;
+
+namespace Ftgo.Auth.Tests;
+
+/// <summary>
+/// End-to-end coverage of the multi-tenant app-only policy used by Ftgo.Restaurants.Api.
+/// Mirrors <see cref="OrdersNamedPoliciesEndToEndTests"/> but for the single-policy (app-only)
+/// resource API shape — proves the policy enforces role + azp allow-list + scp-rejection.
+/// </summary>
+/// <remarks>
+/// This test exists specifically to catch the regression where an author writes
+/// <c>[Authorize(Roles = "Restaurants.Read.All")] [RequireClientApp]</c> instead of using the
+/// named policy. With <c>MapInboundClaims = false</c> (doctrine), the framework looks for
+/// claims of type <see cref="System.Security.Claims.ClaimTypes.Role"/>, but the JWT carries
+/// the role under the short name <c>roles</c>, so the role check is silently skipped — the
+/// endpoint then accepts any allow-listed app regardless of role. The two `_PolicyMustEnforceRole`
+/// tests exercise that exact failure mode.
+/// </remarks>
+public sealed class RestaurantsAppPolicyEndToEndTests : IAsyncLifetime
+{
+#pragma warning disable IDE1006
+    private const string AllowedAppId = "22222222-2222-2222-2222-222222222222";
+    private const string AppPolicy = "RestaurantsApp";
+    private const string Role = "Restaurants.Read.All";
+#pragma warning restore IDE1006
+
+    private IHost _host = null!;
+    private HttpClient _client = null!;
+
+    public async ValueTask InitializeAsync()
+    {
+        _host = await new HostBuilder()
+            .ConfigureWebHost(web => web
+                .UseTestServer()
+                .ConfigureServices(services =>
+                {
+                    services.AddRouting();
+                    services.AddSingleton<IOptionsMonitor<EntraAuthOptions>>(_ =>
+                        new StaticOptionsMonitor<EntraAuthOptions>(new EntraAuthOptions
+                        {
+                            AllowedClientApps = [AllowedAppId],
+                        }));
+
+                    services.AddAuthentication(TestAuthHandler.SchemeName)
+                        .AddScheme<TestAuthSchemeOptions, TestAuthHandler>(TestAuthHandler.SchemeName, _ => { });
+
+                    services.AddAuthorization(o =>
+                    {
+                        o.AddAppPolicy(AppPolicy, Role);
+
+                        // Pin the test scheme onto the doctrine policy so MapGet([Authorize(Policy=...)])
+                        // resolves against the Test handler (mirrors the JWT scheme in production).
+                        var existing = o.GetPolicy(AppPolicy)!;
+                        o.AddPolicy(AppPolicy, new AuthorizationPolicyBuilder(TestAuthHandler.SchemeName)
+                            .Combine(existing)
+                            .Build());
+                    });
+                    services.AddSingleton<IAuthorizationHandler, RequireClientAppHandler>();
+                    services.AddSingleton<IAuthorizationHandler, RequireAppRoleHandler>();
+                })
+                .Configure(app =>
+                {
+                    app.UseRouting();
+                    app.UseAuthentication();
+                    app.UseAuthorization();
+                    app.UseEndpoints(e =>
+                    {
+                        e.MapGet("/system", () => Results.Ok("app")).RequireAuthorization(AppPolicy);
+                    });
+                }))
+            .StartAsync();
+
+        _client = _host.GetTestClient();
+    }
+
+    public async ValueTask DisposeAsync()
+    {
+        _client.Dispose();
+        await _host.StopAsync();
+        _host.Dispose();
+    }
+
+    private static HttpRequestMessage Request(string path, params (string type, string value)[] claims)
+    {
+        var req = new HttpRequestMessage(HttpMethod.Get, path);
+        var encoded = string.Join("|", claims.Select(c => $"{c.type}={c.value}"));
+        req.Headers.Authorization = new AuthenticationHeaderValue(TestAuthHandler.SchemeName, encoded);
+        return req;
+    }
+
+    [Fact]
+    public async Task RestaurantsApp_Allows_AppTokenWithRoleAndAllowListedAzp()
+    {
+        var resp = await _client.SendAsync(
+            Request("/system", ("roles", Role), ("azp", AllowedAppId), ("tid", "tenant-a")),
+            TestContext.Current.CancellationToken);
+        resp.StatusCode.ShouldBe(HttpStatusCode.OK);
+    }
+
+    [Fact]
+    public async Task RestaurantsApp_Rejects_TokenWithScp_MixedClaims()
+    {
+        var resp = await _client.SendAsync(
+            Request("/system", ("roles", Role), ("scp", "restaurants.read"), ("azp", AllowedAppId)),
+            TestContext.Current.CancellationToken);
+        resp.StatusCode.ShouldBe(HttpStatusCode.Forbidden);
+    }
+
+    [Fact]
+    public async Task RestaurantsApp_Rejects_AppToken_WhenAzpNotAllowListed()
+    {
+        var resp = await _client.SendAsync(
+            Request("/system", ("roles", Role), ("azp", Guid.NewGuid().ToString())),
+            TestContext.Current.CancellationToken);
+        resp.StatusCode.ShouldBe(HttpStatusCode.Forbidden);
+    }
+
+    [Fact]
+    public async Task RestaurantsApp_Rejects_UserToken()
+    {
+        var resp = await _client.SendAsync(
+            Request("/system", ("scp", "restaurants.read")),
+            TestContext.Current.CancellationToken);
+        resp.StatusCode.ShouldBe(HttpStatusCode.Forbidden);
+    }
+
+    [Fact]
+    public async Task RestaurantsApp_Rejects_AllowListedAppWithoutRequiredRole()
+    {
+        // Regression guard for the [Authorize(Roles=...)] anti-pattern: with the named policy in place,
+        // an allow-listed app that is MISSING the required role MUST be rejected. If a future author
+        // re-introduces [Authorize(Roles="...")] with MapInboundClaims=false, the silent role-check
+        // failure would let this request through and this test would fail.
+        var resp = await _client.SendAsync(
+            Request("/system", ("roles", "SomeOther.Role"), ("azp", AllowedAppId)),
+            TestContext.Current.CancellationToken);
+        resp.StatusCode.ShouldBe(HttpStatusCode.Forbidden);
+    }
+
+    [Fact]
+    public async Task RestaurantsApp_Rejects_AllowListedAppWithNoRolesClaim()
+    {
+        // Same regression guard: an allow-listed app with no `roles` claim at all must be rejected.
+        var resp = await _client.SendAsync(
+            Request("/system", ("azp", AllowedAppId), ("tid", "tenant-a")),
+            TestContext.Current.CancellationToken);
+        resp.StatusCode.ShouldBe(HttpStatusCode.Forbidden);
+    }
+}