Skip to content

Architecture review: working findings (2026-10-03)

Status: work in progress. This was captured before the cloud session was stopped to save credits. Items marked VERIFIED were confirmed by reading the code in that session. Items marked AGENT were reported by a reviewer subagent and have only been partly spot-checked. Re-check those before acting on them. Delete this file once the review is folded into issues/ADRs (docs/planning/ is temporary).

Resume

  • Continue locally: from a clean worktree of camaradesuk/syrf, run claude --teleport session_01F4EfmSKy87FzqVQeo186c9. Use a scratch worktree so main/ stays on main. The conversation history comes with it; background reviewers and scratch files do not.
  • Still to do:
  • Fold in any reviewer reports appended below: API, PM domain, Identity, CI/CD, and the cross-cutting bug hunt.
  • Verify the AGENT items.
  • Write the prioritised report: weaknesses, refactoring roadmap, modernisation, bugs.

Context

  • One human maintainer plus agents. About 1,250 PRs were merged between 4 Aug and 3 Oct 2026 (#2697 → #3954).
  • Size. About 500K lines of C#. Production code:
Project Lines
PM Core 89.6K
PM Mongo.Data 22K
API 33.9K
Identity 17K
Identity.Migration 11.5K
PM service 12K
PdfAgent 9.5K

The Angular app is 160K lines of TS plus 155K lines of spec. - Statistics code dominates the domain. ProjectStatistics (FEAT-024, dark and default-off) is about 59K production lines, 51K test lines and 17.8K doc lines. It is 50K of PM Core's 89.6K lines (56%).

Verified findings (this session)

Delivery / dependency truth

  1. Four descriptions of the dependency graph have drifted apart. They are the csproj ProjectReference graph, docs/architecture/dependency-map.yaml (which calls itself the "single source of truth" and also drives scripts/generate-dockerfiles.py), the .github/services.json path_filters, and the SERVICE_CONFIG table in .github/scripts/detect-service-changes.sh.
  2. s3-notifier Lambda: it compiles against SharedKernel, WebHostConfig.Common and PM.Messages → PM.Core → AppServices, and uses PM.Core ProjectAggregate types and SharedKernel Settings/Enums/ValueObjects directly (S3FileReceivedFunction.cs:17-22, UploadEventDispatch.cs:8-11). But detect-service-changes.sh:79 only watches src/services/s3-notifier, src/libs/s3-notifier, PM.Messages and Directory.Build.props, and _detect-changes.yml:303 uses that script. Library changes therefore never rebuild or redeploy the Lambda.
  3. dependency-map.yaml is wrong in several places:
    • PM.Messages lists depends_on: [SharedKernel], but the csproj also references PM.Core.
    • s3-notifier lists only S3FileSavedNotifier.Messages.
    • AppServices claims Mongo.Common, FluentValidation and AutoMapper, none of which it uses.
    • nuget.config is listed in the Docker contexts but doesn't exist.
    • pdf-agent is missing.
  4. services.json: quartz path_filters omit src/libs/webhostconfig, which is Quartz's only ProjectReference.
  5. Fix: generate all three from the csproj graph (MSBuild or dotnet list reference), or validate them against it in CI.
  6. Swagger UI publishes the Auth0 Swagger client secret.
  7. Program.cs:785-797 passes SwaggerAuthConfig:ClientSecret into NSwag OAuth2ClientSettings.ClientSecret. The chart defaults to provider: auth0 with secret swagger-auth (api/.chart/values.yaml:218-221, _env-blocks.tpl:538-544).
  8. NSwag's index.html renders clientSecret into initOAuth (checked against the upstream template), and UseOpenApi/UseSwaggerUi are mounted unconditionally and anonymously.
  9. PKCE is already on, so the secret is unnecessary.
  10. Fix: drop the secret (as the OpenIddict path already does) and rotate it.
  11. SignalR EnableDetailedErrors = true is unconditional (Program.cs:381), so exception messages reach clients in production.
  12. UseDeveloperExceptionPage() runs in Staging (Program.cs:765), so stack traces are visible on internet-facing staging.
  13. Exit codes are inconsistent. The API (Program.cs:813-822) and Quartz (Program.cs:118-127) exit with code 0 after a fatal startup exception. PM returns 1, Identity sets ExitCode = 1, and PdfAgent returns 1.
  14. Dead startup code:
  15. The results of container.WhatDoIHave()/WhatDidIScan() are thrown away (Program.cs:742-743).
  16. A manual throw new NullReferenceException (:736-739).
  17. Commented-out AddServiceDefaults/MapDefaultEndpoints.

Composition / DI

  1. Lamar's "last registration wins" ordering is load-bearing. The comments in Program.cs:608-680 explain which registry must come last for the right implementation to win.
  2. Convention scans register dark-feature services in hosts that never call them, and those hosts must then satisfy the services' dependencies to pass AssertConfigurationIsValid.
  3. MongoLamarRegistry.cs:72-79 scans app-base-dir assemblies by name prefix, including the legacy prefixes "Soles" and "C3Rs".
  4. Fix: explicit per-feature AddXxx() modules with TryAdd defaults and an explicit Replace/Decorate for overrides.
  5. "UnitOfWork" is a singleton registered by naming convention (MongoLamarRegistry.cs:82-102). It is a repository facade, not a unit of work.
  6. Static ambient state:
  7. ResourceSecurity.Instance (Program.cs:89).
  8. The settable statics SyrfMapper.Mapper/Config.
  9. HttpHelper.Configure(IHttpContextAccessor) (Program.cs:800).
  10. CA2211 is disabled in .editorconfig.
  11. AutoMapper:
    • The 2014-era "Heroic AutoMapper" scanning pattern is duplicated: App_Start/AutoMapperConfig.cs and Infrastructure/AutoMapperConfig.cs each define an AutoMapperConfigurator.
    • There are 56 static SyrfMapper call sites, and value resolvers do database I/O (IPmUnitOfWork) while mapping, so N+1 queries are hidden there. ConstructServicesUsing uses the root container.
    • AutoMapper 16.1.1 is commercially licensed (v15+) and no LicenseKey is configured. CAMARADES may qualify for the free Community licence.
    • Fix: Mapperly (source-generated) or explicit projections.

Strategic dependency risk

  1. MassTransit 8.4.0. Open-source v8 support ends at the end of 2026; v9 is commercial. Usage is heavy: consumers, sagas, the Quartz scheduler, and both EF and Mongo saga repositories. Pick a path now: license v9, move to the OpenTransit fork, or migrate (e.g. Wolverine or Rebus).
  2. SharedKernel carries infrastructure:
    • It references Lamar, MassTransit, MailKit, AutoMapper, Serilog, Newtonsoft, System.Reactive and Microsoft.AspNetCore.Http.Abstractions 2.2.0, a deprecated 2.x package (Mongo.Common uses 2.3.0).
    • PM Core (the domain) references Bogus, Mvc.NewtonsoftJson, MongoDB.Driver, Polly and Humanizer, and has a UserSecretsId.
  3. Message contracts depend on the domain. PM.Messages references PM.Core. PM.Application is 72 lines in 2 files and looks vestigial.
  4. No Directory.Packages.props, so versions drift:
    • Test packages:
    • xunit 2.6.6 vs 2.9.3, and its runner 2.5.6 vs 3.1.4.
    • Test.Sdk 17.9 / 17.13 / 17.14.
    • Moq 4.20.70 vs 4.20.72.
    • Testcontainers 3.x vs 4.x.
    • coverlet 6.0.0 vs 6.0.4.
    • Other packages:
    • Serilog.Settings.Configuration 8 vs 9.
    • morelinq 4.1 vs 4.4.
    • AspNetCore.Http.Abstractions 2.2 vs 2.3.
    • SDK and tooling:
    • There is no global.json, so the SDK is unpinned on the self-hosted runners.
    • The nswag tool is 14.0.8 while NSwag.AspNetCore is 14.2.0.
  5. Analyzers are mostly switched off but still noisy.
    • AnalysisMode=All and WarningLevel 5 are set, but TreatWarningsAsErrors=false.
    • About 69 rules are muted, including every culture rule (CA1304/1305/1307/1308/1310/1311), CA2211 and CA1031.
    • CS8618 and CA2016 are downgraded to suggestion.
  6. Observability: Elastic APM, Serilog Elasticsearch sinks, Sentry and OpenTelemetry (plus a beta Prometheus exporter) are all referenced. The browser adds Elastic RUM, Sentry and LogRocket session replay, which is a GDPR question.
  7. Email has three paths: SendGrid, AWS SES (with static keys) and MailKit SMTP (to Mailpit).

Hygiene

  1. Committed build output and clutter:
    • src/services/api/SyRF.API.Endpoint/msbuild.binlog (624 KB) is committed, and *.binlog is not in .gitignore.
    • Root clutter: INCIDENT-REPORT.md, PR-DESCRIPTION.md, PROJECT_GUARD_IMPROVEMENTS.md.
  2. Hand-rolled S3 signer. S3PostSigner is AWS sample-code SigV4. Its dead FormatCredentialStringForPolicy returns the AWS-docs example credential (S3PostSigner.cs:26-29). AWSSDK.S3 is already referenced and offers presigned URLs.
  3. API and PM use deploymentStrategy: Recreate (api values.yaml:6, pm values.yaml:5), which means a full outage on every rollout unless cluster-gitops overrides it.
  4. Workflow size. pr-preview.yml is 4,543 lines, ci-cd.yml 3,353, e2e-tests.yml 2,346 and pr-tests.yml 2,233. .github/scripts holds about 70 bash/JS test scripts.
  5. Composition roots are large. API Program.cs is 822 lines, PM 632 and Identity 619. They are full of milestone-coded comments (M1a, M2b, "FEAT-024 wave 1D", "P9 (2026-10-02)", #3866) that will rot.
  6. Dead Helm helper. syrf-common.getValue is unused, and would treat false/0 as missing if it were used. _env-blocks.tpl (2,044 lines) is generated from env-mapping.yaml (2,439 lines).

Appendix A: Web frontend (AGENT report; key bugs VERIFIED)

Verified bugs

  • SignalR loses group subscriptions after an automatic reconnect.
  • withAutomaticReconnect() (signal-r.service.ts:633) gives the client a new connectionId.
  • On disconnect the server clears that connection's project and study subscriptions (NotificationHub.cs:112-122). On connect it re-adds only the user group (:99-104).
  • The client re-subscribes to the project only when combineLatest([projectId, _connected$, isMember]) re-emits. connected stays true through an auto-reconnect: it is set false only by onclose and in the _connect catch (:762, :811), and the ComponentStore select uses distinctUntilChanged, so nothing re-emits.
  • Listings are subscribed only in _connect().then (:759). Export subscriptions use the same gate (:1172-1190).
  • Result: after a network blip, live project, listing and export updates stop silently.
  • Fix: re-subscribe in onreconnected, or use withStatefulReconnect() with AllowStatefulReconnects on the server.
  • Permanent listener after a reconnect. _wireProjectReloadOnReconnect uses switchMap(() => _currentProjectId$) (:1231-1243). After the first reconnect, every change of the current project dispatches another loadProject.
  • Search-import progress and error actions are never dispatched.
  • addSearch$ is declared {dispatch:false} (project-detail.effects.ts:1433-1473), but uploadSearchReferenceFile returns an Observable of NgRx actions (s3-file.service.ts:37-55,151,204). notificationSideEffect$ is only a tap (:104-120).
  • Result: the progress bar stays at 0, and on an S3 failure the dialog stays on "Uploading…".
  • The effect also uses switchMap and throws without a catchError.
  • Operator-precedence bug. search-ui.reducer.ts:50 has props ?? 0 < 100, which always evaluates to uploading.
  • Project navigation race.
  • LoadProjectRequest uses the exhaust strategy (requests.ts:22-25 → exhaustMap, global-request-state.actions.ts:353).
  • The loadProject reducer sets loadedProjectId and replaces projectLoadingState (project-ui.reducer.ts:75-94). detailLoaded then sets loadedProjectId from the response (:95-108).
  • The guard waits with no timeout (project-guard.service.ts:149-185).
  • Result: clicking project B while A is still loading can hang navigation, or flip the "current project" back to A while the URL shows B.
  • Fix: derive the current project from router state and key the load state per project.
  • Request-state reducer bugs. The switch-cancel result is discarded (global-request-state.reducer.ts:194-199), and exhaust triggers are never recorded (:210-213), so their promises never resolve.
  • A second data export is dropped. dataExportReceived$ uses exhaustMap (data-export.effects.ts:119-147), so a second export's download never runs. It should use mergeMap.
  • Unenforced lint and strictness.
  • Production builds use tsconfig.build.json with strict:false, strictNullChecks:false and strictTemplates:false.
  • No workflow and no husky hook runs ng lint, ESLint or a strict tsc, so the eslint-suppressions.json baseline is not enforced in CI.
  • Debug settings in the production bootstrap. main.ts:733-746 has withDebugTracing() on unconditionally, withComponentInputBinding() registered twice, and Store devtools always on with trace:true.
  • Personal data in an unused file. core/state/entities/project/project-detail.viewmodel.stub.ts is 2,387 lines with 0 importers and contains 4 personal email addresses. Delete it, and consider purging it from history.

Other agent findings (not individually verified)

State management. Six idioms coexist: - Global NgRx: 37 hand-rolled normalizr slices and 109 effects. - signalStore ×19. - ComponentStore in 21 files, 10 of them with empty state. - @rx-angular/state. - BehaviorSubjects. - ngrx-forms.

There are also three request-tracking schemes and three RequestStatus types.

God files:

File Lines
stage-review.component.ts (also excluded from Sonar) 2,216
project-detail.effects.ts 1,926
AF2 store 3,036
AF2 component 2,478
signal-r.service.ts 1,635

Structure: - 7 import cycles covering 171 files, caused by core importing types from feature components (e.g. core/state/entities/investigator/investigator.selectors.ts:12 imports admin.component). - About 17 unreferenced components, directives and pipes. - info/contact-us/dynamic-locale.ts duplicates core/dynamic-locale.ts. - core/state/config/features.selectors.ts hand-writes 20 selectors that duplicate generated ones. - state-example.ts is stray.

Effects: - About 20 non-idempotent writes use switchMap: created$, stageAdded$, questionCopied$, inviteMembersResponse$, and in review-effects.ts commit$, deleteSession$ and nextStudy$. - Several effects lack catchError (keywords$ :778-786, the auth social-login effects). - warnOnErrorWithRetryOption$ completes silently on dismiss.

API errors. Effects read err.error, which is undefined on NSwag's ApiException. A single normaliser is needed.

Subscription leaks (constructor subscribes that are never torn down): - study-filters.component.ts:71 - stage-permissions.component.ts:71 - question-selection.component.ts:96,112 - annotation.component.ts:127 - study-table.effects.ts:22-45

Smaller bugs: - .sort() with no comparator (project-index-ui.selectors.ts:48). - jobs.sort mutates a memoized selector result (risk-of-bias-job.selectors.ts:70). - 3 debugger; statements and 91 console.log calls. - 22 selectors throw (throwErrorExp).

Modernisation status: - Done: standalone (1 vestigial NgModule), control flow, lazy routes and functional guards. - Remaining work and counts:

Area Count
@Input vs input() 328 vs 301
@Output vs output() 58 vs 93
Decorator queries 73
Constructor DI params ~400
Explicit Eager CD components (zoneless backlog; zone.js still in polyfills) 167
UntypedForm* 309
Class interceptors 6
ngOnDestroy 49
@HostListener/@HostBinding 54
Files using @ngbracket/ngx-layout 122
  • Schematics exist: signal-input-migration, output-migration, signal-queries-migration, inject, cleanup-unused-imports.

Tests: - 556 specs and 6,132 tests. - 94 spec files contain only "should create". - 62 use NO_ERRORS_SCHEMA; 609 as any and ~351 private-member accesses. - 20 use real-time sleeps. - src/services/web/CLAUDE.md has drifted (OnPush count, ngrx-forms usage, waitForAsync).

Dependencies: - NgRx 21 on Angular 22 (peer overrides). - Both the Auth0 SDKs and angular-auth-oidc-client. - Both the moment and date-fns adapters. - font-awesome 4.7, json-patch 0.7 and normalizr (unmaintained). - Jasmine types alongside Vitest; @types/node 18. - dompurify ">=2.5.9" (open-ended range). - The build needs a 7 GB heap.

Appendix B: Shared libs, Quartz, s3-notifier, pdf-agent (AGENT report; not yet verified)

Bugs

  • B1. Domain events are fire-and-forget.
  • MongoUnitOfWorkBase.cs:737-740 calls _eventManager.DispatchAsync((dynamic) domainEvent, messageSender); and never awaits it. There is no try/catch in EventManager.cs:25-31 and no UnobservedTaskException hook.
  • Handler failures are lost, e.g. InvitationEmailSendRequestedEventHandler throwing on an email failure.
  • Handlers outlive the request, so ordering is lost, the user context is lost, and the IsolatedReadScope they inherit is already disposed.
  • B2. Change-stream health stays unhealthy after recovery (mechanism confirmed, trigger plausible).
  • The retry operator only resets on an event (MongoContext.cs:398-405).
  • The API registers the check with tag live, and /health/live has no predicate (SyrfHealthCheckExtensions.cs:40-43), so pods get liveness-killed after the database has recovered.
  • The 15-retry budget accumulates across successful reconnects. Once exhausted, the cached Publish() subject is terminated permanently.
  • B3. One subscriber exception terminates the shared change stream (plausible).
  • Subscribers run inline (MongoContext.cs:265) and shouldRetry (:306) excludes non-Mongo exceptions.
  • Example trigger: NotificationHub.cs:1163 calls Entity.IsDisabled when Entity is null.
  • B4. Health probes are inverted.
  • /health/live runs every check, while /health/ready runs only ready-tagged checks.
  • Quartz's SQL checks (Quartz/Program.cs:58-59) therefore drive liveness, with timeoutSeconds: 1 (quartz/.chart/values.yaml:155).
  • B5. Orphaned BlockingStream consumer parks a thread forever.
  • PubmedXmlParseImplementation.cs:144-148 and EndnoteXmlParseImplementation.cs:197-202 have no try/finally, and the upload uses CancellationToken.None (S3FileService.cs:61-62).
  • Each failure leaks a thread and an unaborted multipart upload. RisParseImplementation.cs:127-133 does this correctly.
  • B6. Upsert concurrency resurrects deleted aggregates.
  • MongoExtensions.cs:262-299 uses ReplaceOneAsync({_id, Audit.Version}, IsUpsert=true). A deleted document is silently re-inserted.
  • There are 114 Save/SaveAsync calls against 38 uses of TrySaveExistingAsync.
  • B7. Cache reset race (plausible). ResetCache swaps the cache (RepositoryCache.cs:330-346), so a load that started before a save can cache stale data for 2 s, and Dispose can race a reader.
  • B8. Batched saves drop domain events. GetDomainEvents() clears the events (Entity.cs:63-74), and the cancel or failure paths of BatchedSaveManyAsync (MongoUnitOfWorkBase.cs:624-627,671) and of SaveMany(Async) (:506,547) skip dispatch.
  • B9. Every Mongo command is captured into traces. DiagnosticsActivityEventSubscriber(true) (MongoContext.cs:52) writes db.statement = Command.ToString() (DiagnosticsActivityEventSubscriber.cs:117-120). That includes whole Project documents of ~465 KB, and PII.
  • B10. Mongo credentials are not escaped in the mongodb+srv://{Username}:{Password}@ builders (MongoContext.cs:79-81, MongoConnectionSettings.cs:35,63). The two builders also disagree.
  • B11. Quartz exits 0 on a fatal startup error (Program.cs:118-127).
  • B12. Quartz is not clustered (QuartzServiceCollectionExtensions.cs:73-86), and it uses the default RollingUpdate strategy. During a rollout two schedulers can run, causing duplicate triggers.
  • B13. Signing material on stdout. S3PostSigner.cs:115,129,136,143 Console.WriteLines the canonical request, the signature and the Authorization header. S3FileService.WriteStreamToFile uses PublicReadWrite (dead code).
  • B14. Version registry prunes live pods. DatabaseMetadataWriter.cs:200-212 prunes instances whose connectedAt is older than 24 h, and connectedAt is never refreshed (latent until ServiceVersionFloor is used).
  • Minor:
  • MyUtils.GetNthDigit uses 10 ^ 5 as if it were a power (dead code).
  • new BlockingStream() leaves _blocks null.
  • The Lambda creates an S3 client and a MassTransit bus per record (S3FileReceivedFunction.cs:85-110,210-227).
  • s3-notifier/Program.cs:9-11 contradicts the chart's runtime: dotnet10.

Weaknesses

SharedKernel is a grab-bag. - MassTransit and Serilog are referenced but unused. - About 30 of its 189 public types are unreferenced: Maybe, ListSetWithAction, Enumeration, PartitionAsync (global namespace), HttpHelpers, the DataTables view models, and others. - IMessageSender → MessageSender throws NotImplementedException in all three methods (SyrfRegistry.cs:11), yet it appears in 52 signatures. - Duplicates: two RetryWithExponentialBackoff implementations and two GetSyrfAssemblies. - HeaderDictionaryBuilder is in the wrong namespace.

The Lambda and the PDF agent drag in the web and domain stack. - The Lambda gets the whole WebHostConfig (Web SDK, APM, Sentry, OpenTelemetry, Lamar, MassTransit.MongoDb) to use one 74-line RabbitMQ TLS helper. - It gets PM.Core for two enums. - 8 of the 27 PM.Messages files import ProjectAggregate. - The PDF agent is forced onto the aspnet base image. - Fix: split out SyRF.Messaging and PM.Contracts.

Unit of work and repositories. - The singleton UoW takes more than 40 repository constructor parameters. - Reflection runs on every save. - The DispatchProxy rethrows with throw innerException ?? ex;, which loses the stack trace (ActivityAndLogDispatchProxy.cs:64). - The session is applied to the write but not to the read. - The cache ignores the version and session arguments. - RepositoryCache wraps exceptions in InvalidOperationException. - There are 80 RegisterClassMap calls whose ordering is enforced by convention only. - GUID setup is correct for driver 3.x, but it is called from 8 places and there are 62 manual CSharpLegacy sites.

Change-stream fan-out. - Rx Publish().RefCount() runs every subscriber inline in the cursor loop and writes three Info logs per event per subscriber. - There is a dead blocking variant (MongoContext.cs:173-201).

Bootstrap. - Configuration is rebuilt about 7 times, with inconsistent precedence (HostExtensions.cs:96/103 vs :155/156, :190). - Four tracing paths overlap: Elastic APM, OpenTelemetry, Sentry, and an unused ISyrfSpanAccessor. - A stale AddSource name. - Dead code: the Aspire block (the only user of Http.Resilience/ServiceDiscovery), ConfigureSyrfWebHost, and ServiceCollectionExtensions.cs, which is entirely commented out. - No bus-wide retry policy. - Sentry SendDefaultPii = true.

Quartz. - The DbContext sits in the PM namespace. - A static mutable schema field. - About 1,350 lines of Docker provisioning code ship in the production binary. - CORS allows localhost origins with credentials in every environment.

Exemplar. pdf-agent is the cleanest code in this area (ValidateOnStart, TimeProvider, exit codes) and is the template to copy.

Modernisation

  • System.IO.Pipelines instead of BlockingStream.
  • A BackgroundService plus a bounded Channel<T> per change-stream subscriber.
  • BCL replacements: Enumerable.Chunk, ArgumentException.ThrowIf*, Random.Shared, HMACSHA256.HashData, ToLowerInvariant in the SigV4 signer (the Turkish-culture bug), the in-box System.Linq.AsyncEnumerable, and BindingFlags.DoNotWrapExceptions.
  • Use TimeProvider widely. 8 domain events stamp DateTime.Now while 18 use UtcNow, and dispatch sorts on that value.
  • [LoggerMessage] (0 uses today).
  • ValidateOnStart everywhere.
  • MemoryCache per-key eviction with change tokens; HybridCache is rejected per repository-cache.md.

Appendix C: API service (AGENT report; not yet verified unless noted)

The reviewer reproduced some behaviours in a scratch project (Lamar 15.0.1 / ASP.NET 10).

Bugs

  1. The Lamar scan overrides explicit singletons (reproduced).
  2. Program.cs:611-615 runs Scan(TheCallingAssembly + WithDefaultConventions) after the factory registrations at :193-198 (IRuntimeFeatureFlagProvider/Store) and :320 (typed client IBffSessionTokenValidator). Lamar's NewType rule compares only ImplementationType, so the scan appends a transient IFoo→Foo registration, and the last one wins.
  3. Every consumer then gets a new provider. Each constructor (RuntimeFeatureFlagProvider.cs:29-34, ApplyBackendFlags) writes the deployed values into the shared FeatureFlags singleton. Runtime overrides (including the statistics kill switch) are invisible to ReviewController, SubmitAnnotationSessionService, RuntimeProjectStatisticsFlagSource, FeatureGatedAttribute, StageWorkloadShares and StageAllocationRead, and are reverted until the next 30 s refresh.
  4. The BFF validator loses its 5 s timeout (falls back to 100 s) and gets a raw HttpClient per instance.
  5. Fix: explicit registrations or OverwriteBehavior.Never, plus a test that asserts singleton identity.
  6. A project administrator can take ownership.
  7. PATCH api/projects/{id} (ProjectController.cs:365-390) only needs ProjectEditPolicy. ProjectUpdateDto.OwnerId flows through Project.Update to ChangeProjectOwnership (Project.cs:855-866).
  8. ProjectChangeOwnerPolicy is never used by any endpoint.
  9. Update sets Name, IsPublic and other fields before the owner check can throw, leaving a partial mutation.
  10. Mutate the shared cached aggregate, then return 409 without saving.
  11. Endpoints: ApproveJoinRequests :509-519, DeclineJoinRequests :533-544, ResendInvitationsBulk :640-648, RevokeInvitationsBulk :667-674, UpdateInvitationGroupsBulk :688-694, and the UpdateStagePermissions loop. The same pattern appears at NotificationHub.cs:184→273→281 and StudyController.cs:221-246.
  12. RepositoryCache gives the same instance to every caller for 2 s, and AuthorizationHandler.TryGet reads it.
  13. Only 2 BeginIsolatedReads call sites exist in the API.
  14. Forbid(StudyNotInProjectMessage) returns 500 (reproduced; the string is treated as a scheme name). ReviewController.cs:101,315,477,605,677,734. It fires whenever ReviewEligibilityPolicy is off, which is the default.
  15. A 404 written inside AuthorizationHandler breaks the response (reproduced). AuthorizationHandler.cs:154-162 and :349-355 write the body, then Forbid throws "response already started" and the client sees a truncated response. The body isn't valid JSON, and {projectId} is not interpolated.
  16. Anonymous RetrieveInvitation returns 500 because it reads CurrentUserId, which throws for anonymous callers (ProjectController.cs:577-584, SyrfBaseController.cs:9).
  17. Anonymous GetKeywords leaks private projects' keywords. There is no IsPublic filter (ProjectController.cs:92-99 → ProjectRepository.cs:123-128). It is also an unauthenticated full-collection aggregation.
  18. DataExportJob hub subscriptions are never cleaned up on disconnect (NotificationHub.cs:120-121).
  19. Unbounded paging. In GetTableData (StudyController.cs:68-89 → StudyRepository.cs:1286), PageSize=0 means no limit and a negative FromRow causes a 500.
  20. Full-stats push. In AggregateRootEntitySubscriptionManager.cs:329-343,504-516, every study change does a sync Projects.Get plus a full-stats aggregation per subscriber. The unbounded .Merge() can deliver results out of order, so stale stats can win. DateTime.Now is used as DbTime (:348,:523).
  21. Unsynchronised FeatureFlags mutation (RuntimeFeatureFlagProvider.cs:83-88). A flip in the middle of a request can cause an NRE in RemoveSession (ReviewController.cs:154-231); this only applies outside production.
  22. Auth0 token cache compares a UTC expiry with local time (AuthManagementApiClientProvider.cs:33/45).
  23. AnnotationAnswerDtoConverter swallows errors and returns null (AnnotationConverter.cs:320-324).
  24. Liveness includes external dependencies (MassTransit bus, flag reader). /health/live has no predicate (SyrfHealthCheckExtensions.cs:41-44).
  25. Other:
    • Exit code 0 on fatal error.
    • GetProject marks every span as Error (ProjectController.cs:250).
    • Leftover Console.WriteLine timing code (:742-745).
    • First-login race creates the Investigator twice, so the second request gets a duplicate-key 500 (AuthorizationHandler.cs:467-474).
  26. Anonymous endpoints:
    • The contact and support forms send email to any caller-supplied address with a caller-chosen Name and Subject, with no rate limit or CAPTCHA (ApplicationController.cs:200-245). This is an open relay / phishing risk.
    • email-lookup returns investigator GUIDs (AccountController.cs:160-172). Its intended policy uses an "actions" scheme that is never registered (Program.cs:405-411).

Structure

  • Authorization:
  • The global AuthorizeFilter runs every policy twice per request, including a sync Projects.TryGet (AuthorizationHandler.cs:207-214).
  • Three authorization modes coexist: legacy, shadow and enforced, with 32 enforced branches. Production runs legacy.
  • The HTTP and SignalR handlers duplicate the same logic.
  • The ApiKey scheme is registered but never used.
  • Controllers: 43 files, 10.7K lines.
  • Largest: Review 1707 (RemoveSession alone is 239 lines; 20 constructor deps), Project 1364, BulkPdfUpload 1238.
  • 27 of 38 inject IPmUnitOfWork, with 206 direct repository calls, 12 sync Save and 20 sync Get.
  • There are 4 base classes, and about 20 controllers lack [ApiController].
  • 6 inline Polly loops.
  • ICrudRepository has no CancellationToken.
  • 24 InvalidateCache() calls wipe the whole cache when GetUncachedAsync exists.
  • Errors: 4 MVC filters, ad-hoc ProblemDetails, an {error} handler, a plain-text 404, and the developer exception page on Staging. There is no AddProblemDetails.
  • JSON:
  • Newtonsoft is configured twice and has drifted: SignalR is missing AnnotationAnswerDtoConverter.
  • System.Text.Json is used in 9 writers.
  • Mapping:
  • Production uses App_Start/AutoMapperConfig.cs, but tests use SyrfApiMapperProfile, so the tests validate a different configuration.
  • N+1 resolvers.
  • AnnotationSummaryResolver returns hard-coded fake numbers (it is unused).
  • There is no licence key, and NullLoggerFactory hides the licence warning.
  • SignalR: no backplane; per-pod change streams are the design. User and project groups are joined but nothing is ever sent to them.
  • Telemetry and email:
  • Elastic APM (placeholder config), Sentry (off; SendDefaultPii), OpenTelemetry and Serilog console all coexist.
  • Five Serilog Elasticsearch/HTTP packages are unused.
  • The SendGrid package is unused.
  • Mail is sent inline with no outbox (Identity has one).
  • Dead code:
  • 10 fully commented-out files (~440 lines) and Web API 2-era binding models.
  • FileStreaming/Multipart helpers, BadRequestUnhandledExceptionFilter, CustomTokenRetriever, MappingKeys, the Nactem routes.
  • A no-op GET "Generator" endpoint with [FromBody], kept as a codegen hack.
  • 6 of the 7 contracts in SyRF.API.Messages; the living-search events are published but have no consumer.
  • AddResponseCaching is registered but never used.
  • The DataExportDtos exclusion in the csproj points at nothing.
  • The MongoDriver3 alias is used once.
  • Humanizer.Core.uk (Ukrainian resources) is referenced.

Ranked fixes

  1. The Lamar scan.
  2. OwnerId in ProjectUpdateDto.
  3. Isolated reads, and GetUncachedAsync in place of InvalidateCache.
  4. Quick fixes: Forbid, the handler 404, RetrieveInvitation, keywords, the DataExport unsubscribe, the PageSize bound, the liveness predicate, the exit code.
  5. Application services for RemoveSession, screening saves and BulkPdf reconciliation.
  6. Retire the legacy authorization and Auth0 branches.
  7. Delete dead code and unused packages.

Modernisation: - FallbackPolicy. - AddProblemDetails + IExceptionHandler, with [ApiController] everywhere. - The built-in rate limiter. - An immutable per-request flag snapshot. - TimeProvider. - Mapperly, with I/O moved out of resolvers. - System.Text.Json. - A singleton ContentInspector and an injected S3 client.

Appendix D: PM domain and service (AGENT report; not yet verified)

Shape

  • Core is not a pure domain layer. It contains the application services (ProjectManagementService 1662 lines, StageReviewService), Lamar registries, seeding and AutoMapper resolvers. MongoDB.Driver is used in 85 Core files. IClientSessionHandle appears 316 times versus 27 for ISessionHandle, and the README's "framework-agnostic" claim is stale.
  • Distributed monolith. The API (49 files use IPmUnitOfWork, ≥23 direct Project/Study saves) and PM (31 consumers, 1 Mongo saga, ~9 hosted services) both write pmProject, pmStudy and the statistics collections; no collection has a single owner. PM is pinned to replicaCount: 1 + Recreate, and correctness depends on it (the single-writer Bulk PDF endpoint and the fold worker). PM's EF packages are unused.
  • ProjectStatistics:
  • About 50K lines in Core, 6K in Mongo.Data and about 44K in tests.
  • 36 pmProjectStatistics* collections, about 25 flags and 381 ProjectStatistics*-prefixed types.
  • It is roughly 100× the legacy $facet stats (StudyStats.cs, 494 lines).
  • No production acceptance yet (STATUS.md, 2026-10-02).
  • Its registry block is copied 3 times in PM Program.cs (59-93, 175-209, 282-375) and once more in the API.

Bugs

  • B1. The agreement threshold arrives in PM empty (PLAUSIBLE, high).
  • AgreementMeasure.cs:22-23 has protected setters. MassTransit 8.4's System.Text.Json serializer with the default resolver ignores non-public setters, so the consumer receives ProjectAgreementThreshold(null, 0) (UpdateStudyScreeningStatsConsumer.cs:94-95,109).
  • StudyRepository.cs:1626-1646 then adds a junk (0,null) inclusion element to every Study.
  • Verify read-only: pmStudy.countDocuments({"ScreeningInfo.InclusionInfo.ProjectAgreementThreshold.NumberScreened":0}), or a serializer round-trip test.
  • B2. ValueObject equality ignores base-class private fields (CONFIRMED; also found by the bug hunt). ValueObject.cs:54 uses GetFields on the runtime type, so all thresholds compare equal, while GetHashCode (:73) walks the hierarchy. The guards at Project.cs:192,180 and ProjectManagementService.cs:1247 can never trigger, and the assertion at DelegatedWorkProducerTests.cs:286 is vacuous. Fix: records.
  • B3. Event handler failures are lost (CONFIRMED). For example JoinRequestApprovedEventHandler.cs:38 throws but is never awaited. If the send in ProjectAgreementThresholdUpdatedHandler fails, ActiveInclusionInfoCalculationJob stays set and UpdateAgreementMode returns UpdateAlreadyPending forever (Project.cs:1415).
  • B4. Recording a project error always throws (CONFIRMED). UpdateStudyScreeningStatsConsumer.cs:140 adds an anonymous object to List<object> Errors (Project.cs:122). The driver's ObjectSerializer rejects it, so a BsonSerializationException masks the real failure.
  • B5. Direct Study writes don't bump Audit.Version (CONFIRMED path). StudyRepository.cs:1306-1319 ($pull), 1321-1350 (ApplySimpleUpdates) and 1626-1646 (inclusion pipelines) can be silently overwritten by a concurrent whole-document save. For example, deleted-question annotations come back, or an inclusion element is lost.
  • B6. Version is bumped in memory before the write and never rolled back (MongoExtensions.cs:270-271). A failed save leaves the cached instance at N+1, and the next save can overwrite another host's write. There is also a ResetCache race (RepositoryCache.cs:199,208,330-337).
  • B7. A progress-update conflict rolls back the whole import. ProjectManagementService.cs:585-590 passes the handler as onBatchSaved (MongoUnitOfWorkBase.cs:661). It runs a sync UpdateParsingProgress().GetAwaiter().GetResult() under flag.Wait(), and any concurrent Project write raises a duplicate key. :602-604 then deletes every imported study.
  • B8. The inclusion recalculation can latch forever. The consumer has no retry (:154-160) and there is no global retry. The precondition is checked through PM's 2 s cache (:65-68), so on a stale read it throws, B4 follows, the message goes to the error queue, and the project stays latched as CalculatingInclusionInfo.
  • B9. New projects can lose their creator's membership (dormant). Audit.OnSaving ignores schemaVersion (Audit.cs:28-35). With DefaultSchemaVersion=1 (e2etest config), the class map writes only an empty Registrations list (ProjectRepository.cs:1410-1415).
  • B10. Risk of Bias reports success after a partial failure (RobProcessingService.cs:515,453). This is flag-guarded and known.
  • B11. CSV/TSV imports read as lenient UTF-8 (NewSpreadsheetReader.cs:76,94,133), so Windows-1252 characters become U+FFFD. RIS handles this correctly.
  • Minor:
  • _logger.LogWarning(errorMessage, e) loses the stack trace (ProjectManagementService.cs:1528,1549,1590).
  • UpdateStudyInclusionInfoForAllProjectsAsync loads every Project (:1402).
  • Sync-over-async in ThrowIfGuarded and ApplicationService.cs:65,99.

Weaknesses

  • Fire-and-forget domain events. EventManager.cs:21 ignores messageSender, and NullProjectEmailService throws in PM.
  • Unsound concurrency model. There are 3 BeginIsolatedReads call sites against about 99 cached loads that feed about 80 saves, plus 83 whole-type flushes.
  • Project is a god aggregate and a hot document (1725 lines, 118–465 KB). It embeds memberships, invitations, join requests, stages, questions, searches, the state of 5 job kinds and List<object> Errors.
  • Contracts embed domain types (8 of 27 files), and queue names derive from CLR types. Five contracts are dead.
  • No global retry or outbox. About 12 of 31 consumers have retry, and the tests configure retries that production lacks.
  • The saga uses CreatePartitioner(1) (SearchImportJobStateMachine.cs:308).
  • The Parsing state has no timeout (:154-167).
  • v0/v1 dual-schema branching in 7 types (ShouldSerialize at ProjectRepository.cs:1379-1418).
  • Unused dependencies: Bogus, Humanizer, Mvc.NewtonsoftJson and Linq.Async in Core; EF in PM.

Ranked fixes

  1. Await events, then add a MassTransit Mongo transactional outbox.
  2. Use records for value objects and messages, add a System.Text.Json round-trip test for every contract, and use primitive fields in contracts.
  3. Add a global retry/redelivery/outbox callback, give the saga a timeout, and raise the partition count.
  4. $inc Audit.Version on direct Study writes and enforce it with architecture tests.
  5. Use a scoped identity map instead of RepositoryCache, and non-upsert saves.
  6. Move job state out of Project.
  7. Purge dead code:
  8. StudyRepository FilterSortCursor/Cursor 1176-1298, which also has CSV injection.
  9. Dead query methods.
  10. The dead aggregates Potential, ReferenceLibrary, ProjectDailyStat and RiskOfBiasAiJob.
  11. NewEndnoteReader.
  12. Unused packages.
  13. Seeding into its own assembly; fold or delete Application; one AddProjectManagementDomain().
  14. Give ProjectStatistics its own assembly and namespaces, and decide whether to activate it or freeze it. Migrate v0/v1 documents and delete the branches.
  15. Explicit modules with one writer per collection, or accept and simplify the modular monolith.

Modernisation: - Records. - TimeProvider: 34 DateTime.Now uses, and mixed-kind event times are sorted at MongoUnitOfWorkBase.cs:736. - In-box AsyncEnumerable.Chunk and MaxBy to replace MoreLinq and MoreAsyncLINQ. - A Channels progress pipeline (replacing NonSyncProgress, the Timer, SemaphoreSlim and sync waits), and DataExportProgressReporter.Report, which blocks. - Parallel.ForEachAsync. - [LoggerMessage] on hot paths.

Appendix E: Cross-cutting backend bug hunt (AGENT report; not yet verified)

  1. HIGH, CONFIRMED path: invitation token ignored.
  2. Project.cs:1361 is RetrieveInvitationByToken(string token) => PendingInvitations.SingleOrDefault();. The correct GetPendingInvitationByToken exists at :1394.
  3. RespondToInvitation (:1350) is reached via ProjectController.cs:598-608 with dto.Token. The policy is ProjectRequestToJoinPolicy, and AllowAllApplicationUsers: true lets any signed-in user through.
  4. Invitation.Respond/LinkInvestigator (Invitation.cs:185-193,64-78) links any unlinked invitation, and CreateMembership grants its groups.
  5. Impact: any user can POST {"conclusionType":"Accepted","token":"x"} and join a project that has exactly one pending invitation, with that invitation's groups, which may include admin. With two or more pending invitations the call returns 500.
  6. HIGH: email-verification codes can be brute-forced.
  7. AccountController.cs:478-529 and Investigator.cs:120-152,265-292: 6-digit codes valid for 24 h, with no attempt counter. Each resend adds another valid code.
  8. A verified email triggers NewEmailAddressAssociatedWithInvestigatorHandler.cs:20 → ClaimInvitationsForEmailAsync (ProjectManagementService.cs:1075), which claims pending invitations.
  9. The API has no rate limiter (0 hits).
  10. MEDIUM: a global password-attempt budget enables a lock-out DoS. PasswordAttemptStore.cs:20,104-107 allows 120 attempts per minute globally, checked before the per-source limit (:115). About 2 requests/s locks out every password sign-in, and the result looks like a wrong password (ProtectedPasswordSignIn.cs:34).
  11. MEDIUM: rejected mutations leak through the shared RepositoryCache (same as API #3). About 40 load-mutate-save methods lack BeginIsolatedReads.
  12. MEDIUM: ValueObject equality (same as PM B2). UpdateStudyScreeningStatsConsumer.cs:94-110 can apply threshold A and mark job B successful.
  13. MEDIUM: Study-library filters hard-code 2 reviewers. StudyRepository.cs:1665,1669,1712,1725 use >= 2 / < 2 instead of Stage.SessionCountTarget. Single-reviewer and three-reviewer stages show the wrong status.
  14. MEDIUM: import progress. ProjectManagementService.cs:544-567: the Interlocked.Exchange sits outside finally, so one failure freezes progress. A sync handler conflict aborts the import (:584-587).
  15. MEDIUM: one subscriber exception silently kills the shared change stream (MongoContext.cs:265,292). AutoDetachObserver disposes the source, so retry never sees the error and project live updates stop on that pod.
  16. MEDIUM: unauthenticated email relay (ApplicationController.cs:200-245).
  17. LOW-MEDIUM: out-of-order stats. The .Select(FromAsync).Merge() at AggregateRootEntitySubscriptionManager.cs:340,516 should use Switch or Concat.
  18. LOW-MEDIUM: retry helper.
    • MyUtils.cs:603-640 sets retryStartTime once per subscription and never resets retryCount. On connections older than 24 h, the first error ends the stream. After about 10 lifetime errors, every retry waits 10 minutes.
    • The onError handlers rethrow into Rx (AggregateRootEntitySubscriptionManager.cs:86,184,375,457).
  19. LOW-MEDIUM: anonymous keywords. Private projects leak, and a null keyword makes ToDictionary throw, which is a 500 for everyone.
  20. LOW:
    • PendingInvitationAcceptedInstead is not handled (ProjectController.cs:463-474), so the user gets a 500 after the change was saved.
    • ClaimedByAnother returns 500.
  21. LOW: thread-unsafe hash. S3SignerBase.cs:51 holds a static HashAlgorithm used by the singleton signer, so concurrent signing can produce SignatureDoesNotMatch. The signer also logs the signature and Authorization header to stdout (:115-143).
  22. LOW: export status failures are swallowed and reported only through Debug.WriteLine (DataExportProgressReporter.cs:300-329).
  23. LOW: paging.
    • IncomingTableParamsDto.cs:34 clamps to an empty last page.
    • Limit(0) means no limit (StudyRepository.cs:1287).
  24. LOW: lost domain events on cancellation (MongoUnitOfWorkBase.cs:624-627).
  25. LOW: anonymous email-lookup returns GUIDs (AccountController.cs:160-171).

Dormant: - A Descending index overload builds Ascending (MongoExtensions.cs:108-115). - MyUtils.cs:310 XOR. - The S3 canonical URI is not encoded. - RepositoryCache._locks grows without bound.

Hygiene counts (non-test C#):

Pattern Count Notes
async void 0
.Result 34 2 on Tasks, both benign
.Wait() 3
GetAwaiter().GetResult() 22 14 in seeder/startup
DateTime.Now 38 33 in persisted aggregates/events
DateTime.UtcNow 201
Files using TimeProvider 93
catch blocks 851 168 are catch (Exception) without a filter (126 don't rethrow); 79 are bare catch
throw ex 5
static collections 90 all read-only except AnnotationQuestion.cs:454-480
ReplaceOne 37 all version-filtered
UUID( 0
[AllowAnonymous] 27
Enum.Parse 34

Appendix F: CI/CD, charts, repo hygiene (AGENT report; stopped early; not yet verified)

Risks

  1. Review is nominal.
  2. self-approve.yml:9-28: an admin comments /approve and the bot approves. All of the last 50 merged PRs were bot-approved, and the author merges.
  3. The ruleset needs 1 approval, with no stale-review dismissal and no last-push approval.
  4. The default GITHUB_TOKEN is writable, and ci-cd.yml create-tags/create-docker-releases have no permissions: block.
  5. Actions may approve PRs, so a PR workflow running PR code with the default token could approve its own PR (PLAUSIBLE). Example: docs-validation.yml → _validate-docs-indexes.yml runs pip install and the PR's docs/scripts/*.py with no permissions block.
  6. The only required check, "Test Summary", fails open.
  7. pr-tests.yml:1978-1983 skips it when detection doesn't match, and a skipped job counts as Success. #3930 and #3951 merged that way. If detect fails, its outputs are empty and the check is green.
  8. The .NET regex (pr-tests.yml:83) ignores Directory.Build.props, .editorconfig and src/testing/*.cs.
  9. test-ci-cd.yml:479 defines a job with the same name.
  10. validate-workflows, Sonar and docs checks are not required.
  11. Main deploys without full gating.
  12. test-dotnet-integration (ci-cd.yml:951, 25 min timeout) is not a dependency of build-docker (1304-1311), create-tags (1560) or promote-to-staging (2148). It was cancelled in about 25 of the last 60 main runs, but promotions still went ahead.
  13. package-lambda (1440) needs only detect and version, and overwrites production.zip even when tests fail.
  14. Run 37093956306 opened production PR #1536 with ".NET tests" failing.
  15. promote-to-production:
  16. It copies each staging config.yaml wholesale, including deploymentNotification.deploymentId (ci-cd.yml:2805), so the production PostSync hook marks the staging Deployment successful (_postsync-notify.tpl:96-111).
  17. It snapshots cluster-gitops about 4 s after creating the auto-merge staging PR (2686), so production gets the previous staging state.
  18. It runs even when nothing was promoted.
  19. create-lambda-release never works. It uploads /tmp/released.zip (1540) but attaches lambda/production.zip (2141), and with no always() the job is skipped. There are no s3-notifier releases among the last 100.
  20. Change detection misses real dependencies.
  21. PM (detect-service-changes.sh:69) omits src/libs/api and src/libs/s3-notifier, which PM.Endpoint.csproj:35-36 references.
  22. s3-notifier (:75) omits kernel, webhostconfig, appservices and PM.Core.
  23. retag-images can promote a PR build (PLAUSIBLE). ci-cd.yml:1251 git describe --match "${TAG_PREFIX}*" includes prerelease tags, and 173 PullRequestNNNN tags exist in the same GHCR repos.
  24. A failed version job still builds (1318), falling back to '0.0.0' (1342-1352). It pushes 0.0.0 images and moves latest.
  25. The preview DB lock fails open.
  26. _preview-cleanup.yml:88-100 uses gh pr view ... 2>/dev/null || echo "". The token can't read labels, so lock_db=false and the database is dropped.
  27. pr-preview.yml:2093-2116 maps the same failure to drop (:2409-2415).
  28. The label grep is a substring match (:3929-3936).
  29. The lock-db label doesn't exist.
  30. Script injection:
    • docs-rebuild.yml:27,32-34,164,226-227 interpolates client_payload.* into shell.
    • snapshot-on-demand.yml:96,168 interpolates reason in a job holding GKE credentials.
    • e2e-tests.yml:137-138,160 interpolates label names under pull_request_target.
    • All are low-medium risk: triggering needs write access and the repo is private.
  31. No pipefail.
    • Multi-line run steps with pipefail: ci-cd 1 of 42, _preview-cleanup 0 of 20, preview-sweep 0 of 16.
    • The guard at preview-sweep.yml:61-67 can't fire.
    • Scheduled sweeps always dry-run (:41-44), so closed PRs 2230, 2450 and 2851 still own 21, 45 and 29 tags.
    • Across workflows: 112 2>/dev/null, 75 || true, 38 continue-on-error.
  32. CI mutates the cluster directly. pr-preview.yml:1992-2040 strips ArgoCD Application finalizers, which orphans their resources and contradicts the GitOps-only rule.
  33. Credential blast radius.
    • Static AWS keys at ci-cd.yml:1472, pr-preview.yml:1405 and _preview-cleanup.yml:433; in the preview Lambda job they are loaded after PR code is built on a self-hosted runner.
    • The bucket is camarades-terraform-state-aws.
    • Cleanup and sweep fetch the prod/staging-shared Quartz SQL credential and run DROPs, without masking the values.
  34. Possible approval via prompt injection (SPECULATIVE). claude-code-review.yml:269 allows Bash(gh api:*) with a PR-write token while reading PR content.
  35. promote-production.yml:86-92 is stale. It uses the legacy layout, yq@latest, and has never run.
  36. Other:
    • An always-created staging Deployment stays in_progress (ci-cd.yml:2163).
    • The GitVersion regexes accept only [a-z]+ scopes, so feat(bulk-update): gets a patch bump (10 of 67 feat commits), and the ^BREAKING CHANGE: footer never matches.

Weaknesses

  • Size and duplication.
  • 28 workflows, 19.3K lines (about 10.4K of them inline bash); 1 composite action.
  • 27–35% of each big workflow is per-service copies.
  • 4 change detectors; _detect-changes.yml is unused and _gitversion.yml is used only by test-ci-cd.
  • services.json is dead and wrong (a missing Dockerfile.ci; syrf-pm vs syrf-project-management), yet CLAUDE.md cites it.
  • pdf-agent is missing from releases and the run summary.
  • Brittle contract tests. Some pin step-body SHA-256 digests (test-workflow-validator-consolidation.sh:350-366).
  • Check noise. 84–105 check runs per PR head, 59–81 of them skipped.
  • Build warnings. The main build reports 14,220 warnings, about 12.9K of them CA2007 from libs. High-severity NU1903 SSH.NET advisories in test projects are buried in them.
  • Dependency management.
  • No CPM; 12 of 117 packages drift.
  • No global.json; the .NET 8 SDK is installed only for NSwag 14.0.8.
  • No Dependabot or Renovate.
  • No lock files and no NuGet caching.
  • The buildx type=gha cache has no scope (_docker-build.yml:347-348), and the Sonar cache key never changes.
  • Images and charts.
  • Floating base tags, provenance: false, no scanning or signing.
  • The generated s3-notifier Dockerfile is broken (EXPOSE None, missing libs).
  • Web nginx runs as root.
  • The .NET pods have no securityContext.
  • The docs and user-guide charts are copies of each other.
  • Jenkins X leftovers (OWNERS, Kptfile, Helm-2 Makefiles) and Tekton tasks in legacy/.
  • Hygiene.
  • msbuild.binlog (contains C:\Users\chris paths) is committed.
  • docs/planning has 379 files (231 Completed; 256 of the 322 dated ones are more than 90 days old), and .planning has 141.
  • funding/ holds contracts and a payment plan.
  • 12 csproj declare unused Local;Docker configurations.
  • The placeholder docs-backlog-sync.yml has opened 10 issues that were never closed.
  • CLAUDE.md is stale: E2E labels are syrf-e2e-*, and Mongo is 8, not 7.
  • The ci-cd self-hosted lanes skip juniper-job-cleanup.sh.

Ranked fixes

  1. Governance.
  2. Drop self-approve, or set required approvals honestly.
  3. Make Test Summary always run and fail when detection fails, and make the validation summary required.
  4. Dismiss stale approvals.
  5. Read-only token by default, with permissions: {} at workflow level.
  6. Release gate. One gate job that needs all tests, in front of Lambda, retag and promotion. Promote to production only when staging changed, and copy only the tag fields.
  7. One service registry, generated from the csproj graph. It feeds the detect script, the matrices, the Dockerfiles and the path filters.
  8. Move logic out of YAML.
  9. Inline bash into tested scripts or a small TypeScript tool.
  10. Composite actions for GKE auth, GitOps checkout, tool installs and .NET setup.
  11. Split pr-preview.yml.
  12. Modernise.
  13. .NET packages: CPM with transitive pinning, lock files, NU1903/1904 as errors, global.json, .slnx, grouped Renovate updates.
  14. Upgrade NSwag.
  15. Images: SDK container publish with chiseled images, provenance and SBOM.
  16. CI: AWS via OIDC, and per-image buildx cache scope.
  17. Delete dead code and config.

Appendix G: Identity service (AGENT report; not yet verified)

Shape

  • Endpoint: about 15.4K lines. Program.cs is 619 lines with 55 inline Add* calls, and IdentityHostOptions.BindAndValidate is 585 lines (it skips validation in Development). Controllers still read raw IConfiguration in 14 places.
  • OpenIddict 7.4:
  • Flows: code+PKCE, refresh and client-credentials.
  • Token lifetimes: access 1 h, refresh 14 d, id 30 m.
  • There is no revocation endpoint.
  • Access tokens are encrypted, so every API request calls Identity's introspection endpoint.
  • Clients are re-seeded on every start.
  • Key rotation is manual, and KeyMaterialChangeMonitor restarts the pod when cert files change.
  • DataProtection keys live in Mongo.
  • Persistence: AspNetCore.Identity.MongoDbCore 7.0.0 (net6, a single maintainer, creates no indexes; subclassed to add passkeys). There are three MongoClients.
  • Auth0 migration: the Migration project (10.2K lines plus a 740-line Job chart) is still needed until production cutover (S10–S13) and the 28-day fallback are done. Retire it after that.
  • API validation package: IdentityModel.AspNetCore.OAuth2Introspection 6.2.0 is unlisted. Replace it with Duende.AspNetCore.Authentication.OAuth2Introspection.

Bugs and security issues

  • 5.1 HIGH: change password without the current password.
  • The RequiresPasswordReset check is only on GET (ChangePassword.cshtml.cs:41).
  • The POST (:50-77) calls AddPasswordAsync, then falls back to GeneratePasswordResetTokenAsync + ResetPasswordAsync. The path is allowlisted in IdentityAdmissionMiddleware.cs:16.
  • A stolen session can replace the password with no notification and no step-up.
  • 5.2 HIGH: global password budget DoS (same as bug hunt #3). PasswordAttemptStore.cs:107-108 commits the global increment before the per-source check at :115. The 50K-key ceiling (:111-113) also fails closed.
  • 5.3 HIGH: unlimited anonymous registration.
  • There is no rate limit or CAPTCHA on /api/account/signup or /Account/Register.
  • Each new address gets a welcome mail with an attacker-chosen name, and a PM Investigator plus tombstone are provisioned before the email is verified (AccountApiController.cs:298,315).
  • Each request runs up to 3 $expr scans of production pmInvestigator (ProjectManagementClaimSource.cs:41-44,113,171-200).
  • Each costs about 118 ms of hashing against a 200m CPU limit.
  • 5.4 MED-HIGH: verification links built from the Host header. Register.cshtml.cs:268, ExternalLogin.cshtml.cs:385 and VerificationLinkFactory.cs:22-26 use Request.Host, and there are no AllowedHosts. If the ingress forwards a client-supplied Host, the token leaks to the attacker's host.
  • 5.5 MEDIUM: the #3935 bug still exists on Google first sign-in. An exception after CommitClaimAsync deletes the bound account (ExternalLogin.cshtml.cs:369-411 → RegistrationCompensation.cs:373-385), leaving the address permanently RefusedPreviouslyMapped.
  • 5.6 MEDIUM: credential changes need no reauthentication.
  • Adding or removing a passkey, disabling 2FA and resetting the authenticator need only a cookie (Passkeys.cshtml.cs:89,145, TwoFactor.cshtml.cs:126,148).
  • A password reset does not remove passkeys.
  • This enables a pre-hijack chain via squatted registration.
  • 5.7 MEDIUM: Identity sits on every API request. Introspection uses PBKDF2 client authentication (not cached), with a 200m CPU limit and 1 replica.
  • 5.8 MEDIUM: no indexes on NormalizedEmail/NormalizedUserName and no OpenIddict indexes.
  • There is no unique ClientId, so concurrent seeding can create duplicate clients.
  • Nothing prunes tokens or authorizations.
  • 5.9 LOW-MED: PUT/DELETE api/account/profile always return 500 (AccountApiController.cs:511,641). They use the OpenIddict server scheme, which throws ID0002.
  • 5.10 LOW: the recovery-mail rate-limit source is the raw IP, not normalised to an IPv6 /64.
  • 5.11 LOW: UseExceptionHandler("/error") points at a page that doesn't exist, and a non-GUID userId throws inside MongoDbCore.
  • 5.12 LOW: prompt=login loops forever (AuthorizationController.cs:77-96).
  • 5.13 LOW:
  • Production CORS allows localhost:4200 with credentials.
  • The confidential client has localhost redirect URIs.
  • Tokens travel in query strings with a 1-day lifetime.
  • Logout is a GET that revokes nothing.
  • Raw email addresses are logged at 18 sites.
  • 5.14 LOW (PLAUSIBLE): the single global document is a contention hot spot. If previews run in Development, they get the dev exception page, ephemeral DataProtection keys and no config validation.

Weaknesses

  • Registration is copied 3 times:
  • AccountApiController.SignUp (:75-370)
  • Register.cshtml.cs:47-330
  • AdminApiController.AdminSignUp (:877-1000)

The copies share 133 identical lines and have already drifted (link origin). - Synchronous registration side effects run inside the request, where an outbox should be used. - Security rules are re-implemented per entry point: three ways to build links, and inconsistent step-up and source derivation. - Whole-document updates guarded by ConcurrencyStamp. ClearPendingClaimAsync doesn't rotate the stamp. - Hard-coded global budgets that university NATs will hit. - No metrics, tracing or Sentry, although the chart says sentry.enabled: true. - Readiness depends on the PM database. - A tombstone backfill scan runs on every pod start.

Ranked fixes

  1. 5.1, 5.2 and 5.5 (all small).
  2. Indexes, plus pruning with PruneAsync.
  3. One IdentityLinkBuilder built from the issuer.
  4. One PasswordRegistrationService.
  5. Move registration mail to the outbox, and provision PM only after verification.
  6. [Authorize(Policy=...)] on the admin controller.
  7. Split Program.cs into modules and inject options.
  8. Later: an owned user store with partial $set updates.

Modernisation: AddRateLimiter (per-IP on register, signup, login and forgot-password), AddProblemDetails/UseStatusCodePages, ValidateOnStart/[OptionsValidator], AddAuthorizationBuilder, OpenTelemetry metrics, and introspection caching via HybridCache or local validation.

Priority shortlist (for the next session)

Act now: security, verify first

All of these are small fixes.

  1. Invitation token is ignored (Project.cs:1361): any signed-in user can accept a lone pending invitation, which may carry admin groups.
  2. Password change without the current password (ChangePassword.cshtml.cs:50-77).
  3. Global password-attempt budget lets one client lock out every sign-in (PasswordAttemptStore.cs:107-115).
  4. Swagger UI serves the Auth0 client secret (VERIFIED). Remove it and rotate the secret.
  5. Project admin can take ownership through PATCH OwnerId.
  6. Email-verification codes can be brute-forced (no attempt limit), and there is no rate limiting on anonymous endpoints: signup, contact-form relay, keywords, email-lookup. Add AddRateLimiter.
  7. Governance: the bot self-approves PRs, the required check fails open, and main promotes to staging and production without waiting for the integration tests.

Correctness: high value, low effort

  1. Await domain events, then add an outbox. Add global MassTransit retry.
  2. Fix ValueObject equality (use records). Check whether the agreement threshold survives serialization.
  3. SignalR reconnect (VERIFIED): re-subscribe groups after an automatic reconnect.
  4. Web effects (VERIFIED):
  5. addSearch$ is marked dispatch:false.
  6. Export exhaustMap.
  7. Project navigation race.
  8. Run lint and a strict typecheck in CI.
  9. Lamar scan overrides explicit singletons (flags). Liveness probes include external dependencies, and change-stream health never recovers.
  10. Lambda change detection (VERIFIED). Generate one service registry from the csproj graph.

Strategic, decide soon

  1. MassTransit v8: support ends December 2026.
  2. AutoMapper licence: move to Mapperly.
  3. ProjectStatistics: activate it or freeze it, and isolate it in its own assembly (56% of PM Core).
  4. Modular monolith boundaries: one writer per collection, contracts that don't depend on the domain, and job state moved out of Project.
  5. Retire legacy paths: Auth0, the legacy authorization mode and the v0/v1 schema branches, once their cutovers finish.

Coverage and open threads (pick up from here)

Review coverage by area

Area Review status Verified by main session
Web (Angular) Complete Key bugs VERIFIED (Appendix A)
Shared libs, Quartz, s3-notifier, pdf-agent Complete Not verified
API Complete (some items reproduced by the reviewer in a scratch Lamar/ASP.NET project) Only items 2–7 and 9–10 of the verified list
PM domain and service Complete Not verified
Identity Complete Not verified
Cross-cutting bug hunt Complete Not verified
CI/CD and repo hygiene Stopped early. Workflows, governance, release, change detection and previews are covered Only items 1 and 18–21 of the verified list

Never reviewed (no reviewer assigned, or cut short)

  • Helm charts in depth:
  • the per-service .chart/ directories;
  • src/charts/preview-infrastructure, preview-orphan-sweep and syrf-valkey;
  • how the env-mapping.yaml generator is designed.
  • Dockerfiles and local tooling: the Dockerfiles beyond the reviewer's notes, scripts/, tools/, .devcontainer/, process-compose.yaml and local stack bring-up.
  • e2e/ suite: the Playwright suite's design, flakiness and coverage, and the authority/benchmark harnesses.
  • Web app:
  • accessibility;
  • bundle size and performance (the build needs a 7 GB heap);
  • SCSS and the Material 3 theming migration;
  • i18n and locales;
  • the AF2 feature internals beyond the god-file counts.
  • .NET test quality: only line counts were taken (about 143K PM, 60K API, 53K Identity test lines).
  • MongoDB: indexes, query plans and document size growth, and the data-migration strategy beyond the v0/v1 note.
  • user-guide/ and docs/: whether the content is accurate. Only staleness counts were taken.
  • Dependency vulnerabilities (new; not triaged). GitHub reported 170 open Dependabot alerts on main (2 critical, 53 high, 102 moderate, 13 low) when this branch was pushed.
  • Runtime behaviour. The container had no .NET SDK, so nothing was built or run. The figure of 14,220 build warnings comes from CI logs (CI/CD reviewer).

Verification backlog: AGENT HIGH/MEDIUM items, in order

  1. Invitation token ignored (Project.cs:1350-1400). Read the code, then add a unit test that a wrong token is refused.
  2. ChangePassword POST without the current password (ChangePassword.cshtml.cs:41-77). Read the code and add a page-model test.
  3. PasswordAttemptStore increments the global counter before the per-source check (:104-115). Read the code.
  4. Lamar scan overrides explicit singletons (API Program.cs:193-198,320,611-615). Write a container test asserting that the singleton identity of IRuntimeFeatureFlagProvider holds.
  5. Domain events fire-and-forget (MongoUnitOfWorkBase.cs:737-740, EventManager.cs). Read the code.
  6. ValueObject equality ignores base-class fields (ValueObject.cs:48-87). Unit test with two different ProjectAgreementThresholds.
  7. Agreement threshold lost in MassTransit STJ serialization (AgreementMeasure.cs:22-23). Write a round-trip test. Then, read-only on production syrftest: db.pmStudy.countDocuments({"ScreeningInfo.InclusionInfo.ProjectAgreementThreshold.NumberScreened":0})
  8. Health probes. /health/live has no predicate (SyrfHealthCheckExtensions.cs:40-44), and change-stream health never recovers (MongoContext.cs:398-405).
  9. Upsert resurrects deleted aggregates (MongoExtensions.cs:262-299), and direct Study writes don't bump the version (StudyRepository.cs:1306-1350,1626-1646).
  10. Governance.
    • self-approve.yml.
    • The ruleset settings, read through the GitHub API.
    • "Test Summary" skipped means green (pr-tests.yml:1978-1983).
    • test-dotnet-integration does not gate promotion (ci-cd.yml:951,1304-1311,2148).
  11. Project admin can take ownership (ProjectController.cs:365-390 → Project.cs:855-866).
  12. Registration / signup has no rate limit; Host-header verification links (Register.cshtml.cs:268).

Conflicting or unresolved claims to reconcile

  • .github/services.json. The CI/CD reviewer says it is unused (dead), but CLAUDE.md cites it. If it is dead, the path-filter drift noted under verified item 1 doesn't matter, so delete the file and fix CLAUDE.md. Either way, detect-service-changes.sh is the real trigger list, and it is the one with the Lambda and PM gaps.
  • MongoDriver3 extern alias. The API reviewer says it works around MassTransit.MongoDb pulling in the v2 Driver.Core. The libs reviewer says MassTransit.MongoDb 8.4.0 depends on Driver 3.2.1, so the workaround is stale. Settle it with dotnet nuget why <proj> MongoDB.Driver.Core.
  • Production configuration that can only be confirmed in cluster-gitops (it was absent from this checkout):
  • the Swagger provider and secret in production;
  • API, PM and Quartz replica counts and deployment strategies (Recreate vs RollingUpdate; Quartz has no clustering);
  • whether previews or Identity run with ASPNETCORE_ENVIRONMENT=Development or Staging.
  • InternalsVisibleTo SyRF.API.Tests. The API reviewer says it matches the test assembly name, so this is not an issue. Dropped.

Not yet produced

  • The synthesised final report. It should deduplicate the appendices: several findings appear 2–3 times, for example the cache mutate-then-409 pattern, ValueObject equality, keywords, the health probes and the exit codes. It should then rank them into:
  • architecture weaknesses;
  • a phased refactoring roadmap;
  • a modernisation plan for .NET 10 / Angular 22;
  • a bug list with severities.
  • Suggested GitHub issues or epics for each item, and the ADR candidates: MassTransit's future, mapping without AutoMapper, module boundaries and collection ownership, and the ProjectStatistics activate-or-freeze decision.
  • Raw reviewer transcripts were lost when the container stopped. Only the reports captured above and in the session conversation survive.

Verification results (local session, 3 Oct 2026)

Each item below was verified by reading the code, plus a minimal repro, a live check or a production query where stated. These move from AGENT to VERIFIED.

# Finding Result Evidence
V1 Invitation token ignored CONFIRMED Project.cs:1361 returns PendingInvitations.SingleOrDefault() and never reads the token. Path: ProjectController.cs:597-620 (policy RequestToJoin, which has AllowAllApplicationUsers: true in ResourceSecurity.json:144-149) → RespondToInvitation → Invitation.Respond, whose LinkInvestigator links any invitation with Invitee null → CreateMembership → SaveAsync. Exploit: any signed-in user takes a project's single pending, unclaimed invitation and its groups.
V2 ChangePassword POST skips the current password CONFIRMED The RequiresPasswordReset gate is only in OnGetAsync. OnPostAsync calls AddPasswordAsync, then generates its own reset token and calls ResetPasswordAsync. The path is allowlisted in IdentityAdmissionMiddleware.cs:14. Razor antiforgery blocks CSRF, so an attacker needs a live session (stolen cookie, shared machine). Severity medium-high.
V3 Global password budget enables a lock-out DoS CONFIRMED (by design, but abusable) PasswordAttemptStore.cs: the global increment is saved before the source check. An early return inside WithTransactionAsync commits, so denied attempts still consume the global 120/min.
V4 Swagger UI serves the Auth0 client secret CONFIRMED LIVE https://api.syrf.org.uk/swagger/index.html returns 200 with a non-empty 64-character clientSecret (value not recorded). No cluster-gitops override of swaggerAuthConfig; production syncs swagger-auth.clientSecret (plugins/local/extra-secrets-production/values.yaml:114). Rotate the secret and stop passing it to NSwag.
V5 Domain events fire-and-forget CONFIRMED MongoUnitOfWorkBase.cs:737-740 discards the Task returned by DispatchAsync. The dynamic call suppresses CS4014. EventManager.DispatchAsync has no try/catch.
V6 ValueObject equality ignores base-class fields CONFIRMED (repro) A .NET 10 repro of the same shape: typeof(Threshold).GetFields(Instance\|NonPublic\|Public) returns 0 fields, while the base declares 2. So any two ProjectAgreementThresholds are Equals.
V7 Agreement threshold lost in STJ serialization CONFIRMED (repro + prod data), low impact A plain STJ round-trip with the default resolver turns {"r":0.333,"n":2} into R=null N=0. The API and PM bus configs don't use Newtonsoft (only Quartz does, for its own job data). Production syrftest.pmStudy, $sample of 3,000: every study carries the three standard thresholds (precomputed elsewhere), and 1 of 3,000 has the junk (0,null) element. Real, but rare. A full count timed out and was not retried, to spare production.
V8 Lamar scan overrides explicit singletons CONFIRMED (repro on Lamar 15.0.1) AddSingleton<X>() + AddSingleton<IX>(sp => sp.Get<X>()) + Scan(TheCallingAssembly, WithDefaultConventions) makes IX a new transient each resolve (3 constructions in the repro). The API has exactly this shape (Program.cs:193-198, scan at :611-615). The RuntimeFeatureFlagProvider constructor calls ApplyBackendFlags, which resets the shared FeatureFlags singleton to deployed values, so runtime flag overrides and kill switches are undone for interface consumers.
V9 Liveness and readiness inverted CONFIRMED SyrfHealthCheckExtensions.cs:40-43 maps /health/live with no predicate (all checks), while /health/ready only runs ready-tagged checks. The API and Quartz charts point liveness at /health/live with timeoutSeconds: 1, and there is no cluster-gitops override. A dependency blip restarts pods instead of just marking them unready.
V10 Governance CONFIRMED (live settings) The Main Protection ruleset requires 1 approval, with dismiss_stale_reviews_on_push=false and require_last_push_approval=false. The only required check is Test Summary (integration 15368, so any GitHub Actions job with that name satisfies it). The repo's default workflow token is write and can_approve_pull_request_reviews=true. self-approve.yml lets an admin /approve via the bot. Test Summary is skipped when nothing is detected, or when detection fails and its outputs are empty (pr-tests.yml:1978-1983), and a skipped check counts as passed.
V11 Main promotes without full gating CONFIRMED, worse than reported Dependency closure of ci-cd.yml: test-dotnet-integration gates nothing. package-lambda depends on no tests. build-docker requires the unit tests to pass, but promote-to-staging accepts build-docker == skipped. So when test-dotnet fails, the build is skipped, promotion still runs, and promote-to-production follows on staging success.
V12 Project admin can take ownership CONFIRMED PATCH api/projects/{id} needs only ProjectEditPolicy (Administrators group E432B8B3…, ResourceSecurity.json:95-102). Project.Update (Project.cs:855-866) calls ChangeProjectOwnership when OwnerId differs, and the only guard is active membership. ChangeOwner is owner-only (ResourceSecurity.json:59-64), but ProjectChangeOwnerPolicy is declared (Activity.cs:65) and used by no endpoint. Name, IsPublic and the other fields are mutated before the owner check can throw.
V13 Upsert resurrects deleted aggregates CONFIRMED (mechanism) MongoExtensions.SaveAsync uses ReplaceOneAsync(GetWriteFilter(id, version), IsUpsert = true), and GetWriteFilter is {_id, Audit.Version} plus guards (:513-528). If the document was hard-deleted after it was loaded, the filter misses without a duplicate-key error and the replace re-inserts it. 106 unit-of-work Save/SaveAsync call sites vs 38 TrySaveExistingAsync (non-upsert). Impact depends on hard-delete paths, e.g. import rollback deleting studies.
V14 Forbid(StudyNotInProjectMessage) returns 500 CONFIRMED ReviewController.cs:101,315,477,605,677,734 pass a message string (:1698) to ControllerBase.Forbid(params string[] authenticationSchemes). ASP.NET treats it as a scheme name and throws, giving 500 instead of 403.
V15 Anonymous keywords leak private projects CONFIRMED [AllowAnonymous] GET api/projects/keywords (ProjectController.cs:92-99) → ProjectRepository.GetKeywords (:123-128) aggregates every project's keywords with no IsPublic filter, materialising the whole collection's keyword set. A null keyword would make ToDictionary throw (500 for everyone).
V16 Anonymous contact and support forms relay email CONFIRMED ApplicationController.cs:199-245 ([AllowAnonymous]) sends SendContactUsConfirmation(callerEmail, callerName, callerSubject). There is no rate limiter anywhere in the backend (0 matches for AddRateLimiter/EnableRateLimiting).
V17 Email-verification codes can be brute-forced CONFIRMED Investigator.cs VerificationCode: RandomNumberGenerator.GetInt32(100000, 1000000), 24 h expiry, NewEmailVerifier.Verify checks for a match against all issued codes, and each resend appends a code. There is no attempt counter and no rate limiting. With k codes outstanding, one guess succeeds with probability k/900,000 (20 codes means about 45K guesses on average). A verified email then claims that address's pending invitations (ClaimInvitationsForEmailAsync). The attacker needs only an account.
V18 Study-library session filter hard-codes 2 reviewers CONFIRMED StudyRepository.GetSessionFilter (:1653ff) uses NumberOfCompletedCandidateSessions >= 2 / < 2, while the review-eligibility filters use scope.SessionCountTarget (ReviewEligibilityPoolFilters.cs:84,108,223). Single-reviewer and three-reviewer stages get wrong Completed/In-progress filtering.
V19 Identity verification links built from the Host header CONFIRMED defect, LOW exploitability Register.cshtml.cs:268 and ExternalLogin.cshtml.cs:385 use Request.Host. However, ConfigureForwardedHeaders (Program.cs:477-504) only honours X-Forwarded-Host equal to the issuer host, and ingress routes by Host, so injection is unlikely. Fix for consistency (build links from the issuer), not urgently.

Still unverified: - upsert resurrection (V-backlog 9); - the governance claims (10; need ruleset reads via gh api); - project-admin ownership takeover (11); - the registration rate-limit and Host-header claims (12); - the remaining AGENT items in the appendices.

Owner decisions (2026-10-03)

  • Auth0 is left as it is. The Swagger UI client-secret exposure (V4) is handed to the OpenIddict migration session and fixed there, staging first: issue #3992; code change parked as draft PR #3966. The review session does not rotate the secret or merge #3966.
  • Invitation acceptance keeps the email-association flow (V1 / #3967). The earlier "accept with the token returns 404" claim was wrong: Accept/Decline only appear for an invitation already linked to the signed-in account; an unlinked invitation asks the user to associate the email first. Token-bearer acceptance is not adopted.
  • Release gating: test-dotnet-integration blocks production promotion only (#3971 production-gate); staging stays on release-gate (unit + web tests).
  • Shipped 2026-10-03: #3968 (change-password gate), #3970 (per-source password budget), #3967 (invitation token; closed #2729) and #3971 (release gate + production gate; first main run 37127224233 is the gates' first live execution).
  • No promotion to production without explicit approval (2026-10-03). Findings: the pipeline never merges production itself (promote-to-production only opens a requires-review cluster-gitops PR; production ArgoCD auto-syncs on merge), but cluster-gitops Main Protection required 0 approvals with no required checks, workflow tokens could approve PRs, and 564 open production promotion PRs had accumulated (one per main push). Controls applied: cluster-gitops#1555 adds CODEOWNERS for syrf/environments/production/, plugins/local/extra-secrets-production/ and argocd/projects/syrf-production.yaml; the ruleset now requires a code-owner review (approval count stays 0 so staging auto-merge is unaffected) and dismisses stale approvals on push; can_approve_pull_request_reviews is off. Superseded production PRs closed; the newest is kept as a draft. ci-cd change to a single rolling draft production PR: syrf#3993. Caveat: the ruleset has an "always" bypass for the camarades-argo-cd App, which is also the App ci-cd uses to author/auto-merge staging PRs, so the pipeline token could bypass the review rule; today its code never merges production. Removing the bypass is the owner's call (risk: staging auto-merge may block on unresolved bot review threads).
  • Open: #3969 vs #3964 for the ownership-transfer fix (V12); owner to pick one.