Fix the Demo container and code; add a Demo README and a Docker CI job

[neotrow]

Summary

Makes the Demo buildable, runnable and honest.

• Dockerfile rewritten as a repo-root-context multi-stage build on sdk:10.0 / aspnet:10.0: copies global.json, nuget.config, Directory.*.props, the csproj + packages.lock.json files, restores in --locked-mode, publishes the Demo; runtime stage listens on 8080 (Cloud Run), runs as the non-root app user, and the ENTRYPOINT finally names the assembly that exists. Repo-root .dockerignore keeps bin/, obj/, .git/, node_modules/ out of the context.
• CI gains a docker job that builds the image, starts it with placeholder identity settings, curls / and checks the container runs unprivileged — so the image cannot silently rot again.
• AppDbContextFactory: the sync-over-async CreateDbContext() and the unregistered IDbContextFactory<T> implementation are gone; a blank connection string is treated as missing; the request's CancellationToken flows through.
• /test logs the exception and returns a generic problem response instead of the raw message.
• Csag.AzureSqlFederatedIdentity.Demo/README.md: how to run it locally (ADC, user-secrets / env vars with the exact keys) and on Cloud Run, the SQL to create TestTable (the app's identity normally has only reader/writer rights, so schema is not auto-created), the least-privilege IAM role bound on the service account, and the docker build/run commands.
• Empty changeset — the package is unchanged.

Stacked on the runtime PR.

Review findings addressed

1, 2 (plus .dockerignore, the Demo's sync-over-async path, the exception leak, and the missing schema/run story).

Validation

Proven on Docker 29.7.2 from the repo root:

• docker build -f Csag.AzureSqlFederatedIdentity.Demo/Dockerfile -t csag-demo . ✅
• docker run with the three placeholder identity settings → GET / returns the greeting ✅; id -u inside the container is 1654 (non-root) ✅

Note: the library validates the identity settings at startup, so the container needs Csag.AzureSqlFederatedIdentity__TenantId, __ClientId and __Google__ServiceAccountEmail set to start — the README says so, and the CI smoke test passes placeholders.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Unresolved moderate findings affect CI enforcement and endpoint error/cancellation handling.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Updates the Demo for .NET 10 container deployment, safer request handling, documented usage, and Docker CI coverage.

Changes:

• Adds a multi-stage, non-root Docker image and root .dockerignore.
• Improves database context creation, cancellation handling, and error responses.
• Adds local/Cloud Run documentation and Docker smoke-test automation.

File summaries

File	Reviewed changes	Final review findings
Csag.AzureSqlFederatedIdentity.Demo/README.md	Documents configuration, schema, Docker, and Cloud Run usage.	Foreground Docker command blocks the following curl (nit, 1 vote); ADC mount may be unreadable by UID 1654 (nit, 1 vote).
Csag.AzureSqlFederatedIdentity.Demo/Program.cs	Adds cancellation propagation and generic problem responses.	Factory resolution can bypass endpoint error handling (moderate, 1 vote); expected cancellations are logged as errors and returned as 500 responses (moderate, 2 votes).
Csag.AzureSqlFederatedIdentity.Demo/Dockerfile	Defines the .NET 10 multi-stage, non-root image.	No final findings.
Csag.AzureSqlFederatedIdentity.Demo/Database/AppDbContextFactory.cs	Removes sync-over-async creation and validates connection settings.	Constructor validation can bypass endpoint logging and generic error handling (moderate, 1 vote).
.github/workflows/ci.yml	Adds Docker build and smoke testing.	Smoke test does not verify the effective non-root UID (moderate, 3 votes).
.dockerignore	Excludes build artifacts and unnecessary repository content.	No final findings.
.changeset/demo-fixes.md	Records the Demo maintenance changes.	No final findings.

Review details

Suppressed comments (4)

Csag.AzureSqlFederatedIdentity.Demo/Database/AppDbContextFactory.cs:20

• Because IAppDbContextFactory is resolved as a Minimal API handler parameter, this constructor runs before the lambda enters its try. With the shipped empty connection string, this new exception therefore bypasses the endpoint's LogError and Results.Problem() (and Development's exception page can expose the raw message). Resolve the factory inside the try, or move this validation into code called from inside it.

            var connectionString = configuration.GetConnectionString("DefaultConnection");
            if (string.IsNullOrWhiteSpace(connectionString))
            {
                throw new InvalidOperationException("Connection string 'DefaultConnection' not set.");

Csag.AzureSqlFederatedIdentity.Demo/Program.cs:18

• IAppDbContextFactory is resolved by Minimal API before the handler body runs. Because its constructor now throws for a missing or blank connection string, that failure occurs before this try, so the endpoint neither logs it nor returns the documented generic problem response (and Development can expose the exception). Resolve the factory inside the try, or move the validation into CreateDbContextAsync.

app.MapGet("/test", async ([FromServices] IAppDbContextFactory dbFactory, [FromServices] ILogger<Program> logger, CancellationToken cancellationToken) =>

Csag.AzureSqlFederatedIdentity.Demo/README.md:72

• This command runs the container in the foreground, so the shell blocks at docker run and never reaches the following curl. Run it detached (with a container name), wait for readiness, and stop it after the request so the documented verification is actually executable.

docker run --rm -p 8080:8080 \

Csag.AzureSqlFederatedIdentity.Demo/README.md:78

• The bind mount preserves the host file's ownership and mode, while the image runs as uid 1654. A normal gcloud auth application-default login credentials file is commonly owner-only (0600), so this exact command can make /test fail because /adc.json is unreadable. Document a safe readable copy/ACL for uid 1654 (rather than mounting the private file as-is) before presenting this as the local Docker command.

  -v "$HOME/.config/gcloud/application_default_credentials.json:/adc.json:ro" \
  -e GOOGLE_APPLICATION_CREDENTIALS=/adc.json \

• Files reviewed: 7/7 changed files
• Comments generated: 2
• 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]

🟡 Changes recommended

Factory validation can bypass generic error handling, and the documented IAM and execution flows need correction.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (4)

Csag.AzureSqlFederatedIdentity.Demo/Program.cs:18

• IAppDbContextFactory is resolved for this [FromServices] parameter before the endpoint delegate enters the try. With the new whitespace validation, a missing or blank DefaultConnection therefore throws during parameter binding and bypasses both LogError and Results.Problem (and can expose the exception under Development). Resolve the factory inside the try, or move this validation to code executed inside it.

app.MapGet("/test", async ([FromServices] IAppDbContextFactory dbFactory, [FromServices] ILogger<Program> logger, CancellationToken cancellationToken) =>

Csag.AzureSqlFederatedIdentity.Demo/README.md:78

• The bind mount does not make the ADC file readable by the image's app user. gcloud auth application-default login normally creates this file mode 0600 for the host user, while the container runs as uid 1654, so this command fails with a permission error on a typical Linux workstation before authentication starts. Please document and implement a secure UID/temporary-file ownership workaround instead of only noting the requirement.

  -v "$HOME/.config/gcloud/application_default_credentials.json:/adc.json:ro" \
  -e GOOGLE_APPLICATION_CREDENTIALS=/adc.json \

Csag.AzureSqlFederatedIdentity.Demo/README.md:72

• docker run is foreground here, so the shell blocks at this line and never executes the following curl in the same code block. Run detached with a readiness retry (and stop it afterward), or explicitly tell the reader to use a second terminal; otherwise the documented Docker run sequence cannot be followed as written.

docker run --rm -p 8080:8080 \

Csag.AzureSqlFederatedIdentity.Demo/README.md:57

• This code block also starts dotnet run in the foreground, so the next curl is not reached when the commands are pasted into one shell. Add an explicit second-terminal instruction or provide a background/readiness/cleanup sequence so the documented source-run flow is executable as written.

dotnet run --project Csag.AzureSqlFederatedIdentity.Demo

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

[Copilot]

🟡 Changes recommended

The README’s local execution examples and ADC Docker usage need correction.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

Csag.AzureSqlFederatedIdentity.Demo/README.md:57

• dotnet run keeps the shell occupied, so the next curl command in this same code block is not executed. Show the commands in separate terminals (or otherwise background and wait for the server) so the documented source run actually reaches /test.

dotnet run --project Csag.AzureSqlFederatedIdentity.Demo

Csag.AzureSqlFederatedIdentity.Demo/README.md:72

• This docker run is foregrounded, so it blocks at the container command and the following curl is never reached unless the user opens another terminal. Make the documented sequence runnable by starting the container detached (and stopping it after the request), or explicitly put the curl in a second-terminal example.

docker run --rm -p 8080:8080 \

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

[Copilot]

🔵 Needs a closer look

The changes span Docker, CI, runtime, and database behavior, and readiness assessments are not unanimous.

Review details

Suppressed comments (1)

Csag.AzureSqlFederatedIdentity.Demo/README.md:80

• docker run is also foreground by default, so this curl is never executed when the shown block is pasted into one shell; the documented Docker test therefore cannot be run as written. Either add a detached/readiness flow, or explicitly mark this request as being run from a second terminal.

curl http://localhost:8080/test

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

[Copilot]

🟢 Approval recommended

No unresolved issues were identified in the reviewed changes.

Review details

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