Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates server-side service configuration to filter out noisy Antiforgery logs from the file logger and simplifies data protection configuration by removing explicit key persistence and application name settings. Sequence diagram for filtered Antiforgery loggingsequenceDiagram
participant RequestPipeline
participant Antiforgery
participant LoggerFactory
participant LoggingBuilder
participant FileLogger
RequestPipeline->>Antiforgery: ValidateRequestAsync
Antiforgery->>LoggerFactory: CreateLogger(Microsoft.AspNetCore.Antiforgery)
LoggerFactory->>LoggingBuilder: ApplyFilters
LoggingBuilder->>FileLogger: AddFileLogger
LoggingBuilder->>FileLogger: AddFilter(Microsoft.AspNetCore.Antiforgery, LogLevel.None)
Antiforgery->>FileLogger: Log(LogLevel.Warning, category Antiforgery)
FileLogger-->>Antiforgery: Discard log (filtered out)
RequestPipeline->>LoggerFactory: CreateLogger(OtherCategory)
LoggerFactory->>FileLogger: Log(LogLevel.Error, category OtherCategory)
FileLogger-->>RequestPipeline: Write entry to log file
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The logging filter currently suppresses all logs from
Microsoft.AspNetCore.Antiforgery; consider narrowing this to specific log levels or event IDs so you don’t lose potentially useful antiforgery diagnostics. - Switching from a configured
DataProtectionkey store to the defaultAddDataProtection()behavior changes how keys are persisted; verify that the new behavior (e.g., key lifetime and storage location) still fits your requirements for auth cookies and other protected data across app restarts.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The logging filter currently suppresses all logs from `Microsoft.AspNetCore.Antiforgery`; consider narrowing this to specific log levels or event IDs so you don’t lose potentially useful antiforgery diagnostics.
- Switching from a configured `DataProtection` key store to the default `AddDataProtection()` behavior changes how keys are persisted; verify that the new behavior (e.g., key lifetime and storage location) still fits your requirements for auth cookies and other protected data across app restarts.
## Individual Comments
### Comment 1
<location path="src/BootstrapBlazor.Server/Extensions/ServiceCollectionExtensions.cs" line_range="22-25" />
<code_context>
- // 增加错误日志
- services.AddLogging(logging => logging.AddFileLogger());
+ // 增加错误日志，并过滤已知的防伪 Token 解密噪音日志
+ services.AddLogging(logging =>
+ {
+ logging.AddFileLogger();
+ logging.AddFilter("Microsoft.AspNetCore.Antiforgery", LogLevel.None);
+ });
</code_context>
<issue_to_address>
**issue (bug_risk):** Consider narrowing the antiforgery logging filter instead of disabling the category entirely.
Setting `Microsoft.AspNetCore.Antiforgery` to `LogLevel.None` suppresses all current and future antiforgery warnings/errors, which may hinder diagnosing production issues. Prefer raising the level (e.g., to `Warning`/`Error`) or filtering only the specific noisy event IDs/messages so critical antiforgery failures remain visible in logs.
</issue_to_address>
### Comment 2
<location path="src/BootstrapBlazor.Server/Extensions/ServiceCollectionSharedExtensions.cs" line_range="101" />
<code_context>
- services.AddDataProtection()
- .SetApplicationName("BootstrapBlazor")
- .PersistKeysToFileSystem(new DirectoryInfo(Path.Combine(AppContext.BaseDirectory, "keys")));
+ services.AddDataProtection();
// 增加 PetaPoco ORM 数据服务操作类
</code_context>
<issue_to_address>
**issue (bug_risk):** Reconsider removing explicit DataProtection app name and key persistence configuration.
Removing `.SetApplicationName("BootstrapBlazor")` and `.PersistKeysToFileSystem(...)` means DataProtection will use host defaults for app isolation and key storage. This can result in ephemeral keys (invalidating auth/antiforgery cookies on restart) or unintended key sharing between apps, especially in scaled or long‑running deployments. If this configuration isn’t being set elsewhere, it’s safer to keep an explicit application name and persistent key store.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| services.AddLogging(logging => | ||
| { | ||
| logging.AddFileLogger(); | ||
| logging.AddFilter("Microsoft.AspNetCore.Antiforgery", LogLevel.None); |
There was a problem hiding this comment.
issue (bug_risk): Consider narrowing the antiforgery logging filter instead of disabling the category entirely.
Setting Microsoft.AspNetCore.Antiforgery to LogLevel.None suppresses all current and future antiforgery warnings/errors, which may hinder diagnosing production issues. Prefer raising the level (e.g., to Warning/Error) or filtering only the specific noisy event IDs/messages so critical antiforgery failures remain visible in logs.
| services.AddDataProtection() | ||
| .SetApplicationName("BootstrapBlazor") | ||
| .PersistKeysToFileSystem(new DirectoryInfo(Path.Combine(AppContext.BaseDirectory, "keys"))); | ||
| services.AddDataProtection(); |
There was a problem hiding this comment.
issue (bug_risk): Reconsider removing explicit DataProtection app name and key persistence configuration.
Removing .SetApplicationName("BootstrapBlazor") and .PersistKeysToFileSystem(...) means DataProtection will use host defaults for app isolation and key storage. This can result in ephemeral keys (invalidating auth/antiforgery cookies on restart) or unintended key sharing between apps, especially in scaled or long‑running deployments. If this configuration isn’t being set elsewhere, it’s safer to keep an explicit application name and persistent key store.
There was a problem hiding this comment.
Pull request overview
This PR targets reducing log noise in the BootstrapBlazor.Server sample by filtering Antiforgery-related logs during service registration.
Changes:
- Added a logging filter to suppress
Microsoft.AspNetCore.Antiforgerylogs. - Simplified Data Protection registration by removing explicit application name and key persistence configuration.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/BootstrapBlazor.Server/Extensions/ServiceCollectionSharedExtensions.cs | Removes explicit Data Protection key ring persistence/application name configuration and leaves default AddDataProtection() behavior. |
| src/BootstrapBlazor.Server/Extensions/ServiceCollectionExtensions.cs | Adds Antiforgery log filtering alongside existing file logger setup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // 增加密钥存储配置 | ||
| services.AddDataProtection() | ||
| .SetApplicationName("BootstrapBlazor") | ||
| .PersistKeysToFileSystem(new DirectoryInfo(Path.Combine(AppContext.BaseDirectory, "keys"))); | ||
| services.AddDataProtection(); |
There was a problem hiding this comment.
services.AddDataProtection() used to set SetApplicationName("BootstrapBlazor") and persist the key ring to a known path. This PR removes that configuration entirely, which can change where keys are stored and how antiforgery/data-protection payloads are decrypted across restarts/instances. If the intent is only to reduce Antiforgery log noise (per PR title/issue), consider reverting this change or explicitly documenting/configuring the new key storage strategy (e.g., still set an application name and persist keys to a stable, writable location).
| @@ -99,9 +98,7 @@ public static IServiceCollection AddBootstrapBlazorServices(this IServiceCollect | |||
| services.AddBootstrapBlazorRegionService(); | |||
|
|
|||
| // 增加密钥存储配置 | |||
There was a problem hiding this comment.
The comment says "增加密钥存储配置" (add key storage configuration), but the code now only calls AddDataProtection() without configuring key persistence/application name. Either update the comment to reflect the new behavior or keep an explicit key storage configuration so the comment and behavior stay aligned.
| // 增加密钥存储配置 | |
| // 增加数据保护服务 |
| services.AddLogging(logging => | ||
| { | ||
| logging.AddFileLogger(); | ||
| logging.AddFilter("Microsoft.AspNetCore.Antiforgery", LogLevel.None); |
There was a problem hiding this comment.
AddFilter("Microsoft.AspNetCore.Antiforgery", LogLevel.None) suppresses all antiforgery logs. That can hide potentially important warnings (e.g., repeated invalid token submissions) in addition to the known decryption noise. Consider filtering to a higher minimum level (e.g., LogLevel.Error) or filtering only the specific event/message you want to suppress, so security-relevant antiforgery signals are still available.
| logging.AddFilter("Microsoft.AspNetCore.Antiforgery", LogLevel.None); | |
| logging.AddFilter("Microsoft.AspNetCore.Antiforgery", LogLevel.Error); |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7830 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 764 764
Lines 34142 34142
Branches 4701 4701
=========================================
Hits 34142 34142
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Link issues
fixes #7829
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Adjust logging and data protection configuration for the BootstrapBlazor server application.
Enhancements: