Skip to content

fix(security): protect discovery and expiration log privacy - #813

Merged
mathrmartins merged 3 commits into
developfrom
feature/security-discovery-expiration-log-privacy-20261011
Oct 13, 2026
Merged

mathrmartins merged 3 commits into
developfrom
feature/security-discovery-expiration-log-privacy-20261011

Conversation

@mathrmartins

@mathrmartins mathrmartins commented Oct 11, 2026 •

Copy link
Copy Markdown
Collaborator

Discovery and expiration logging privacy

Source fixes

This four-file draft is based directly on develop at c67f654be8122eecac5fa4ec432a36ef8401f688 and addresses retained GitHub code-scanning alerts #948 and #949, plus the adjacent logout logging path. Signed head: 881e9440376cfa9fce6e55144223473c9cd3b5a4; tree 58c9a4114ae3a19b5db3d019f44378acbd8a435f.

  • OIDC discovery logs the provider count without the caller email or derived domain. Domain extraction, finder input, matched provider list and response remain unchanged, including no-at-sign/multiple-at-sign/case/whitespace inputs.
  • Missing OIDC end-session endpoints retain local logout and the existing null response without logging the supplied provider slug. Advertised endpoint/escaped post-logout redirect behavior and cancellation/error propagation remain unchanged.
  • Permission-expiration queue failure logs retain kind, permission ID, channel and tenant correlation without recipient user ID or arbitrary error description/code. Both channels, priorities, recipient addresses, notification/reference metadata and nonthrowing failed/null-result handling are unchanged.

Regression coverage and checks

  • Added 22 cases: 10 discovery input/match combinations, 2 missing endpoint/private alias cases, 1 advertised endpoint/escaped redirect control, 8 failed/null-result × channel × expiration-kind cases, and 1 successful two-channel delivery control. Tests assert both formatted and structured logger values and exception operands; provider/delivery calls and their original arguments are checked.
  • Pure Roslyn syntax and whole-file token checks passed for all four prepared/applied files. Every nonlogging production token is exact; 24 existing test/helper methods and all pre-existing declarations/imports/fields are exact. Patch apply/whitespace checks passed; signed commit and ordinary remote head readback are exact.
  • Separate boundary/compatibility investigation and bypass/regression review were performed without agents. No source-backed bypass or regression remained after review.
  • Current-head hosted API/architecture/security tests, CodeQL analysis and complete Codacy inventory are pending. No local SDK/product/database/Testcontainers runtime was executed. No scanner rule/configuration, ignore, suppression or dismissal was changed. No alert is certified closed by this draft or by a count/state change.

Preserved scope and evidence

The four original source/test files are byte-identical between the initial inspected e3c639a3 snapshot and current base c67f654b. No new worktree, cleanup, service/container/process changes, force push or default merge was performed. Earlier PR #811 and its complete 138-ID/198-block static proof remain separate; its exact d61edd3 API run38146945068 succeeded with 14,375 tests, 56 privacy cases and both typed migration cases, and is not transferred to this head.

The full goal remains active. Retained Codacy scope4338 observed IDs/historical ignored33252, GitHub494 retained IDs and previous failures remain preserved. Default/historical/ignored/dismissed scope remains uncertified.

Followup after the first current-head CI and CodeQL results

Current signed head: 8ea3a96 (tree 3fee476713cc9c75b35fc532865851975573af1f).

  • The first API job 114500739606/run38149794235 on 881e944 failed: 1405 passed, nine newly added expiration cases failed before the handler ran because Castle cannot proxy ILogger for an internal generic handler. The exact-source failing TRX artifact and log are retained. This is not counted as runtime acceptance.
  • Replaced the logger mock with a test capture of level, event ID, structured properties, exception and formatted text. All eight failure/null-result cases and the success case keep their delivery and privacy assertions. The 13 OIDC cases are unchanged; all 22 still require current-head hosted runtime proof. Production accessibility and assembly/dependency configuration are unchanged.
  • C# CodeQL analysis 1931905499 on 881e944 had one PR-diff result, #956: the source is the enum constant NotificationChannel.Email, not a recipient address or provider error. That result remains tracked. The log boundary now projects only fixed public channel labels (in-app, email, other), while recipient details remain in the unchanged notification call. No alert ignore/dismissal or query setting change was made.
  • Pure Roslyn syntax/token verification passed for the two followup files. All business tokens outside the logger/public-label projection and 14 unrelated API test/helper methods are preserved. This is not a product runtime check.
  • Full six-tool Codacy inventories at base develop c67f654 and initial PR813 head 881e944 both had 3799 unique findings: no removed or added IDs. Source-linked security work here does not claim a Codacy count reduction.

Current-head API, required API/Auth/Authz suites, all 22 regressions and the new CodeQL rescan remain pending. The default branch alerts #948/#949 remain open; this draft PR is unmerged. No global certification, default closure credit or ignored/dismissed closure credit is asserted.

Current logger capture revision

Current signed head: 7419e63, tree 1579742a16ce244a1146d1ad87a8ad20cd980227. The followup uses captured scope state and excludes LogLevel.None. Both affected tests now also assert no emitted scopes; all existing delivery and privacy assertions and all 22 cases are preserved.

Codacy reported two S1172 warnings on the prior 8ea3a96 capture: ea1e1ca106b529564a3799d51ae25442 and 568bf7205bfb07cc74700d0787623baa. Exact current-head six-tool inventory is complete/stable at 3799, both IDs are absent, no new IDs, and the check 114510216116 is successful. The IDs remain in the audit history; source/token evidence links the change to scope capture and real level filtering. No rule, ignore or dismissal change was made. The inventory equals the 3799-ID original develop baseline.

Current API job 114509950227 / run38152885206 and C# CodeQL job114509795774 / run38152883320 remain in progress at this observation. The preceding 8ea API job was cancelled by the newer source event; it is retained without acceptance credit. The initial 881 API failure and #956 result remain preserved. Current runtime and current source-linked CodeQL acceptance are still pending; default alerts #948/#949 remain open and the PR unmerged.

Verified current-head results — 2026-10-11

Signed head 7419e63 / tree1579742a16ce244a1146d1ad87a8ad20cd980227.

  • PR Verify run38152885206 completed SUCCESS. API job114509950227 passed8378 tests in four TRX: API1414, Authentication3099, Authorization2260, SharedKernel1605. Every touched test project and all AGENTS.md minimum suites are present. The unchanged Notifications module is outside this PR's touched-project scope; its delivery calls are preserved and exercised by nine API regressions.
  • All22 new cases passed with exact counts: discovery10, unadvertised logout2, advertised redirect1, failure/null delivery8, successful delivery1. Both recipient/delivery-channel checks, priorities/references, formatted/structured privacy assertions and no-scope assertions passed. Logger IsEnabled reports false for LogLevel.None.
  • Actual API checkout2c0b924ac11fe75a0b3042c438f5ccdff73706db has the exact current source tree. Artifact11707393370 (2122092bytes/SHA73ac356567650f3ae53a0eaf9b64ab62e00902fa3f138098c7b6de05ebff9551) verified head/run/size/digest/ZIPCRC. Full-application OpenAPI artifact11707628099 (117285bytes/SHA342b9141f62100f2ff0d7e2c09a18f7e3470608434e06a1485624ca3e7d2d4d1) also verified.
  • CodeQL run38152883320 SUCCESS: all four current-head language analyses have0 PR-diff results. C#analysis1932005501 scanned6872/6872 files; actual CodeQL checkout is7419e630 with the exact current source tree. C#SARIF has0results. #956 remains in the retained history with provider PR-instance state fixed; no dismissal or ignored-only closure credit.
  • Codacy check114510216116 SUCCESS; complete current-head inventory3799, with the two transient S1172 warnings corrected and no new IDs against the original develop baseline. Their IDs remain retained.

These results verify this four-file source batch on this exact PR head. Default #948/#949 remain open while the draft PR is unmerged. All previous failures and snapshots are retained. No global/default certification or ignored/dismissed closure credit is claimed.

@cla-bot cla-bot Bot added the cla-signed label Oct 11, 2026
@codacy-production

codacy-production Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 22 complexity

Metric Results
Complexity 22

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@mathrmartins
mathrmartins marked this pull request as ready for review October 13, 2026 20:30
Copilot AI balanced review requested due to automatic review settings October 13, 2026 20:30
@mathrmartins
mathrmartins merged commit dced0a8 into develop Oct 13, 2026
24 checks passed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@mathrmartins
mathrmartins deleted the feature/security-discovery-expiration-log-privacy-20261011 branch October 13, 2026 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants