Skip to content

M005 Deferred Items

Campaign checkpoint diagnostic polish after #3214

  • #3261 tracks bounded stale-control and incompatible-dry-run diagnostic categories. Existing paths already refuse safely; this is operator clarity, not an acceptance/security blocker.
  • Merge #3214 first, then add category/privacy regression tests in a diagnostic-only PR. Preserve fresh run/ledger reads and completion compare-and-swap fences.

Closed by the 2026-09-15 sweep (recorded 2026-09-21)

Twelve auth PRs merged on 2026-09-15 (#3219, #3220, #3215, #3229, #3216, #3233,

3214, #3222, #3237, #2928, #2932, #3221). No entry is deleted; this records

which are now resolved and which stay open. Code evidence only — nothing was deployed.

  • Resolved: the Guid.TryParse(externalSubjectId) entry in ResolveInvestigatorIdAsync — SyRF.ProjectManagement.Core/Services/ApplicationService.cs:126-129 now throws InvestigatorMappingRequiredException for a GUID subject without a user_id claim, and :115-120 rejects an invalid claim. The entry's fallback no longer exists.
  • Already resolved, unchanged: "Resolved in PR #3214" (2026-09-05).
  • Stays open: "Campaign checkpoint diagnostic polish after #3214" — tracked as #3261.
  • Stays open: "The OTLP exporter endpoint is still unset in GitOps" — no exporter endpoint has been configured in cluster-gitops.
  • Not re-assessed by this sweep: the build/documentation warning entries, the seeder localhost URIs entry and the GitOps-prune/identity-job-status.sh entry.

Pre-existing build warnings observed during S01

  • SyRF.SharedKernel emits existing nullable, obsolete API, documentation, and analyzer warnings during the identity solution build.
  • SyRF.Identity.Migration emits existing CA1052, CA1862, and CA1851 warnings outside the S01 changes.
  • NuGet intermittently reports NU1900 while loading vulnerability data for the unchanged SharedKernel project from configured package sources. The S01 identity audit nevertheless completed and reported no vulnerable packages for all five projects.

These warnings did not reduce the 314-test baseline and were not changed because they are outside the authentication migration runtime/dependency slice.

Pre-existing build warnings observed during S02

  • SyRF.Identity.Endpoint.Tests retains existing CA1866 warnings in login/change-password/external-login page tests and CA1001 in AuthorizationControllerTests.
  • NuGet intermittently reports NU1900 while loading configured vulnerability feeds for the unchanged SharedKernel project.

These warnings did not reduce the 218-test endpoint baseline and were not changed because they are outside the Identity configuration, ingress, and readiness slice.

Pre-existing documentation validation warnings observed during S02

  • The validation environment does not provide yq, so the script reports that YAML validation is limited while its built-in frontmatter checks still pass.
  • docs/how-to/local-mongodb-development.md has the pre-existing zenhub-ticket: '2380' value instead of an accepted #2380 form.

Documentation validation completed successfully with these two warnings; neither file or tool is part of S02.

OpenIddict seeder localhost redirect and logout URIs deferred to the cleanup slices (2026-08-04)

  • SyRF.Identity.Endpoint/Services/OpenIddictClientSeeder.cs registers localhost redirect and post-logout redirect URIs alongside the transitional SPA URIs. These predate this branch (commit 9978a8247, already on main) and were deliberately left untouched by the Phase 5 readiness work, which changes no client-registration behaviour.
  • Exposure is bounded rather than open: the client is Confidential with PKCE required, so an authorization code delivered to a localhost listener cannot be redeemed without the client secret.
  • Removing them here would widen the readiness slice's blast radius past "no runtime auth path changes" for no gate. They are removed together with the other SPA transition redirect URIs in the S27/S28 decommission cleanup, which is what the inline comment in the seeder already anticipates.

GitOps prune of a rendered operation Job destroys identity-job-status.sh evidence (2026-08-04)

  • scripts/auth-migration/identity-job-status.sh treats a completed Job as the evidence for its operation: it discovers Jobs by the fixed app.kubernetes.io/name / auth.syrf.org/environment / auth.syrf.org/operation selector and fails with "Job discovery is missing" once the object is gone.
  • Raising campaign.ttlSecondsAfterFinished to 604800 (7 days) stops Kubernetes TTL garbage collection from removing earlier operations mid-sequence, but it does not protect against ArgoCD. Each GitOps commit renders exactly one operation Job, so syncing the next operation prunes the previous one regardless of its TTL.
  • S12 must therefore collect the per-operation evidence JSON after each sync rather than relying on a single multi-operation query at the end of the sequence. The raised TTL and this per-sync collection are complementary: the TTL covers the manual canary confirmation and stop/resume gaps within one rendered operation, the per-sync collection covers the prune between operations.

The OTLP exporter endpoint is still unset in GitOps, so no BFF telemetry window can run yet (2026-08-04)

  • AddOpenTelemetryConfig (SyRF.WebHostConfig.Common/SyrfConfigureServices.cs:81-82) builds the resource and the API registers the SyRF.API.BffAuth meter (SyRF.API.Endpoint/Program.cs:81-83), but no OTLP exporter endpoint is configured anywhere in cluster-gitops — a repository-wide search for OTEL/otlp/OpenTelemetry across the GitOps repo returns nothing.
  • S05-SUMMARY.md:253 records this deliberately: "Later rollout slices must configure the existing BFF flag/provider/authority/session generation and OpenTelemetry exporter."
  • Consequence: scripts/auth-migration/query-auth-telemetry.sh cannot return data for any non-baseline mode until the exporter is wired, so S11's first BFF window is blocked on that rollout slice rather than on the harness.
  • The environment dimension itself is no longer part of that gap: BffAuthTelemetry now emits a bounded deployment.environment instrument tag (normalised by the OTLP-to-Prometheus mapping to the deployment_environment label the query script selects on), so the selector holds however the exporter is eventually configured. A resource attribute would not have been sufficient — resource attributes land on target_info, not on each series, unless the collector is configured to promote them.

Resolved in PR #3214 — campaign-final and import-rerun command/catalogue paths (2026-09-05)

  • campaign-final now reconciles the mounted export and current PM mappings against immutable import security provenance, invokes the import verifier, checks settled delivery evidence and calls CampaignRunStore.CompleteRunAsync. The disabled chart operation renders that actual CLI action with private inputs and an aggregate report sink.
  • The earlier claim that import-rerun needed only a catalogue entry was incorrect: the importer also needed actual mutation accounting. The chart now renders import --require-no-writes; a readable zero-write declaration is required before import. A valid declared rerun executes the importer and fails if it creates or repairs documents, without rolling those changes back.
  • See the checkpoint contract for the bounded evidence, non-sending behavior and reviewed operation sequence. This resolves the repository implementation deferral, not the still-required S12 live execution, inspection or retained operational evidence. No production campaign or cutover is implied by these tests.

Guid.TryParse(externalSubjectId) in ResolveInvestigatorIdAsync needs a deliberate design decision (2026-08-04)

  • ApplicationService.ResolveInvestigatorIdAsync (SyRF.ProjectManagement.Core/Services/ApplicationService.cs:115-119) falls back, when the trusted user_id claim is absent or non-GUID, to Guid.TryParse(externalSubjectId) and treats the parsed value as an Investigator id — calling RecordSignInAsync with it and creating the Investigator if none exists.
  • Under Auth0 that fallback was unreachable in practice: an Auth0 subject is auth0|…/google-oauth2|… and never parses as a GUID. Under OpenIddict it is reachable for the first time, because AuthorizationController.Userinfo sets sub to ApplicationUser.Id — a GUID the importer mints with Guid.NewGuid() (UserImporter.cs:92) — and emits user_id only when SyrfUserId is populated. A migrated user with no matched Investigator therefore reaches the fallback with a GUID that is an ApplicationUser id, not an Investigator id.
  • The two outcomes are opposite in kind: either an orphan Investigator is created under the ApplicationUser's GUID, or the create collides on the user's email. Neither is obviously wrong — this may well be the intended new-user path, where a self-registered OpenIddict user legitimately becomes a new Investigator keyed on its own subject.
  • Flagged during round-5 review of PR #2691 and deliberately not fixed there: the round-5 change on this path narrows to RecordExternalSubjectId leaving a migrated Investigator's auth0|… provenance intact, and changing the fallback's identity semantics as well would decide a product question inside a review correction.
  • Needs an explicit decision before the production BFF switch: confirm whether the GUID-subject fallback is the sanctioned new-user path (and if so, cover it with a test that says so), or replace it with an explicit sub-keyed lookup so an ApplicationUser id can never be read as an Investigator id.