Document usage end to end; least-privilege setup guide; CONTRIBUTING and SECURITY

[neotrow]

Summary

Documentation for the API as it stands after the runtime PR.

• Package README (ships in the nupkg) is now self-contained: absolute links only, and a complete quickstart — install → appsettings.json with every option → the three registration overloads → obtaining the token from IAzureSqlTokenProvider and assigning SqlConnection.AccessToken, plus an EF Core factory modelled on the Demo → prerequisites and notes (credential-free connection string with Encrypt=True; the Google ID token is minted via Application Default Credentials, so the runtime identity must be the configured service account). The features list describes what the background refresh actually does.
• Root README: project links (including the Demo README), quick start, contributing pointer; release-process section kept.
• docs/cloud-identity-setup.md: least privilege — roles/iam.serviceAccountOpenIdTokenCreator bound on the service account (the library only calls generateIdToken), with the gcloud command and why a project-level binding is too broad; enable the IAM Service Account Credentials API; the audience is api://AzureADTokenExchange, not the Entra application; database roles as a starting point to narrow; a "Configure the application" section with the exact keys; consistent Microsoft Entra ID naming.
• New: CONTRIBUTING.md (build/test on both TFMs, changesets, 0.x bump convention), SECURITY.md (private vulnerability reporting — now enabled on the repository), .github/PULL_REQUEST_TEMPLATE.md.
• Changeset: patch (the README is part of the package).

Stacked on the Demo PR (it links the Demo README).

Review findings addressed

21, 22, 24, 25, 26, 27 (and the setup guide's copy of 23).

Validation

• Every code sample checked against the actual members and overloads on this branch; every relative link resolves; every absolute URL returns 200 (except the nuget.org page of the new package ID, which exists once the first Csag.* release is published).
• Google role/permission facts checked against Google's IAM documentation.
• Independently verified by an adversarial review (one missing using in a sample was caught and fixed).

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Five unresolved documentation nits remain in the package README and cloud setup guide.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Documentation PR covering package usage, cloud identity setup, contribution guidance, security reporting, and release metadata.

Changes:

• Expands root and package README quickstarts.
• Adds least-privilege cloud identity and Azure SQL setup guidance.
• Adds contribution, security, PR template, and changeset documentation.

File summaries

File	Summary
SECURITY.md	Adds vulnerability reporting policy.
README.md	Updates project overview and quickstart.
docs/cloud-identity-setup.md	Documents cloud setup and least-privilege permissions. Final nits: clarify startup refresh behavior (3 votes, line 199) and scope schema execution grants (1 vote, line 136).
Csag.AzureSqlFederatedIdentity/README.md	Adds package usage documentation. Final nits: narrow hosted-service registration wording (2 votes, line 23), mark overloads as alternatives (1 vote, line 100), and qualify the no-network-round-trip claim (2 votes, line 117).
CONTRIBUTING.md	Adds build, test, and contribution guidance.
.github/PULL_REQUEST_TEMPLATE.md	Adds a pull request checklist.
.changeset/docs-usage-and-guides.md	Declares the packaged documentation patch release.

Review details

Suppressed comments (2)

Csag.AzureSqlFederatedIdentity/README.md:103

• These two overloads are invoked sequentially in one service collection, even though they are alternatives. A copy-paste user can therefore combine multiple options configurators and silently override values; mark the calls as alternatives or put them in separate snippets.

services.AddAzureSqlFederatedIdentity(configuration);

// Sets the options in code; the section name is available as AzureSqlFederatedIdentityOptions.ConfigurationSectionName.
services.AddAzureSqlFederatedIdentity(options =>

docs/cloud-identity-setup.md:136

• GRANT EXECUTE ON SCHEMA::dbo grants execute on every routine in the schema, including future ones, so it contradicts the guide's least-privilege recommendation and the claim that these are object-scoped grants. Scope this example to a named procedure, or remove it unless the application needs schema-wide execution.

GRANT EXECUTE ON SCHEMA::dbo TO [<app-registration-display-name>];

• Files reviewed: 8/8 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

Two moderate documentation issues remain unresolved regarding hosted-service replacement and background-refresh timing.

Review details

Suppressed comments (2)

Csag.AzureSqlFederatedIdentity/README.md:23

• This says every service is registered with TryAdd, but the registration code uses AddHostedService<AzureSqlTokenRefreshService>() for the refresh service. A consumer cannot replace that hosted service just by registering an implementation first, so the feature list overstates the replacement guarantee; limit the statement to the replaceable token-pipeline services (or document how to replace the hosted service).

- Every service is registered with `TryAdd`, so you can replace any part of the pipeline, including `IAzureSqlTokenProvider`, by registering your own implementation first.

docs/cloud-identity-setup.md:199

• With the default EnableBackgroundRefresh=true, AzureSqlTokenRefreshService performs the first token exchange as soon as the host starts (see Csag.AzureSqlFederatedIdentity/Internal/Services/AzureSqlTokenRefreshService.cs:79-104), before any database access. Only when background refresh is disabled does the first database call trigger the exchange, so this flow description can mislead operators about when startup failures and latency occur. Please make the two cases explicit.

At startup the application validates its configuration: the three required settings must be present and `RefreshAheadWindow` must be positive. On the first database access it requests an ID token for the configured service account through ADC (as the runtime service account), exchanges it at Microsoft Entra ID, and opens the connection with the resulting access token. If something fails, the application log names the failing step; the troubleshooting table in the [package README](../Csag.AzureSqlFederatedIdentity/README.md) maps the usual messages to their cause.

• Files reviewed: 8/8 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Two documentation clarifications remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

Csag.AzureSqlFederatedIdentity/README.md:117

• The hosted refresh task is scheduled at the refresh boundary, but a request can arrive in this window before that task runs (or while it is delayed), even when background refresh is enabled and healthy. The parenthetical incorrectly makes this fallback sound limited to disabled or failing background refresh; remove that qualification so the documented behavior matches the implementation.

Resolve `IAzureSqlTokenProvider` (namespace `Csag.AzureSqlFederatedIdentity.Abstractions`), call `GetAzureSqlAccessTokenAsync` and assign the result to `SqlConnection.AccessToken` before opening the connection. Do this for every new connection: until the held token enters the refresh-ahead window the call returns it without any network round trip; inside the window (reachable only when background refresh is off or has been failing) the first caller exchanges a new token and concurrent callers wait for that one exchange.

• Files reviewed: 8/8 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Two moderate and three nit review comments remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

Csag.AzureSqlFederatedIdentity.Demo/README.md:87

• The PR description calls out consistent Microsoft Entra ID naming, but this Demo README still uses the old “Azure AD” name in its introduction, prerequisites, and settings table. Please update those remaining labels as part of this documentation pass so the linked end-to-end guide does not contradict the stated terminology.

Push the image to Artifact Registry and deploy it with the runtime service account set to the configured Google service account and the four settings supplied as environment variables. Cloud Run sends traffic to port 8080, which is the port the image listens on. The [setup guide](../docs/cloud-identity-setup.md) covers the Cloud Run configuration in detail.

Csag.AzureSqlFederatedIdentity/README.md:103

• The overload example invokes both registration methods in one code block. If copied as written, the later code-based Configure runs after the configuration bind and overwrites the configured tenant, client, service account and other values with the placeholders, so these need to be presented as alternatives with an explicit instruction to call only one.

services.AddAzureSqlFederatedIdentity(configuration);

// Sets the options in code; the section name is available as AzureSqlFederatedIdentityOptions.ConfigurationSectionName.
services.AddAzureSqlFederatedIdentity(options =>

Csag.AzureSqlFederatedIdentity/README.md:31

• This prerequisite only says to create the app-registration user, but that CREATE USER ... FROM EXTERNAL PROVIDER step also requires the logical SQL server to have a managed identity with directory-read permissions (for example Directory Readers or the documented Microsoft Graph permissions). Without that setup the stated prerequisite fails with the principal-not-found error. The linked setup guide explains it, but this README is described as self-contained and is the documentation shipped in the nupkg, so include the prerequisite here or explicitly make the setup guide a required dependency.

- **Azure SQL:** a contained database user for the app registration (`CREATE USER [...] FROM EXTERNAL PROVIDER`) with the permissions your application needs.

• Files reviewed: 8/8 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

Documentation and repository guidance are complete with no unresolved review issues.

Review details

• Files reviewed: 8/8 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Cloud Run environment-variable guidance and the schema-wide database grant need correction.

Review details

Suppressed comments (3)

Csag.AzureSqlFederatedIdentity/README.md:75

• The README lists Csag.AzureSqlFederatedIdentity__... as a generic environment-variable form, but this form is not usable on Cloud Run because the section name contains . and Cloud Run rejects such environment-variable names. Since Cloud Run is one of the documented runtime prerequisites, add a Cloud Run-safe configuration alternative or explicitly warn that these names require a different configuration source there.

Environment variables follow the usual .NET mapping, with `__` as the section separator: `Csag.AzureSqlFederatedIdentity__TenantId`, `Csag.AzureSqlFederatedIdentity__ClientId`, `Csag.AzureSqlFederatedIdentity__Google__ServiceAccountEmail`, and so on.

docs/cloud-identity-setup.md:186

• These environment-variable names contain . in the section name, but Cloud Run rejects environment variable names that are not valid identifier-style names. Consequently the documented gcloud run deploy --env-vars-file env.yaml fails before the application starts. Please use a Cloud Run-compatible configuration path (for example, an env-safe section/alias or a mounted configuration/secret file) and update these names and the binding instructions accordingly.

Csag.AzureSqlFederatedIdentity__TenantId: "<tenant-id>"
Csag.AzureSqlFederatedIdentity__ClientId: "<client-id>"
Csag.AzureSqlFederatedIdentity__Google__ServiceAccountEmail: "<name>@<project-id>.iam.gserviceaccount.com"

docs/cloud-identity-setup.md:138

• This example grants EXECUTE on every procedure and function in the dbo schema, which conflicts with the preceding least-privilege guidance and the claim that these grants target particular objects. A consumer copying it would give the app more database capability than intended; use an object-level grant for a named procedure, or clearly label the schema-wide grant as a broader alternative.

GRANT EXECUTE ON SCHEMA::dbo TO [<app-registration-display-name>];

• Files reviewed: 8/8 changed files
• Comments generated: 0 new
• Review effort level: Lite