Skip to content

Add TrustServerCertificate to DefaultConnection#152

Open
jSylvestre wants to merge 1 commit into
masterfrom
JCS/FixSqlExpress
Open

Add TrustServerCertificate to DefaultConnection#152
jSylvestre wants to merge 1 commit into
masterfrom
JCS/FixSqlExpress

Conversation

@jSylvestre
Copy link
Copy Markdown
Member

@jSylvestre jSylvestre commented Apr 17, 2026

Updates the local SQL Express connection string to include TrustServerCertificate=True, resolving SSL/TLS connection issues common in newer versions of the SQL Server driver.

Summary by CodeRabbit

  • Chores
    • Updated database connection settings to improve connectivity reliability.

Updates the local SQL Express connection string to include TrustServerCertificate=True, resolving SSL/TLS connection issues common in newer versions of the SQL Server driver.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 17, 2026

📝 Walkthrough

Walkthrough

Updated the SQL Server connection string in application settings to include TrustServerCertificate=True, enabling encrypted communication with the local SQL Express database instance while maintaining existing connection parameters.

Changes

Cohort / File(s) Summary
Configuration Update
src/tacos.mvc/appsettings.json
Added TrustServerCertificate=True to the ConnectionStrings.DefaultConnection SQL Server connection string to enable secure server certificate validation.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

🐰 A hop, a skip, a config twist,
Security won't be missed!
Trust the cert, encrypt with care,
Our database connection's fair! 🔐

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add TrustServerCertificate to DefaultConnection' accurately and specifically describes the main change: adding TrustServerCertificate parameter to the SQL connection string configuration.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch JCS/FixSqlExpress

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/tacos.mvc/appsettings.json`:
- Line 3: The base ConnectionStrings entry "DefaultConnection" currently
includes TrustServerCertificate=True; remove that token from the base
appsettings.json value so the production/default config enforces strict TLS, and
add a development-only override in appsettings.Development.json (or an
environment-specific override) that sets the same "DefaultConnection" value but
includes TrustServerCertificate=True for local SQL Express testing; ensure the
key name "DefaultConnection" is identical between files so configuration
fallback works.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0a137ebf-cf3d-42e2-bfe1-ab238c11ff97

📥 Commits

Reviewing files that changed from the base of the PR and between 94510e4 and cbf8070.

📒 Files selected for processing (1)
  • src/tacos.mvc/appsettings.json

{
"ConnectionStrings": {
"DefaultConnection": "Server=.\\sqlexpress;Database=tacos;Trusted_Connection=True;MultipleActiveResultSets=true"
"DefaultConnection": "Server=.\\sqlexpress;Database=tacos;Trusted_Connection=True;TrustServerCertificate=True;MultipleActiveResultSets=true"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Scope TrustServerCertificate=True to development-only config

Line 3 weakens TLS identity validation globally when kept in base appsettings.json. Since this PR targets local SQL Express, move this flag to appsettings.Development.json (or an environment override) and keep the base connection string strict.

Suggested config split
# src/tacos.mvc/appsettings.json
- "DefaultConnection": "Server=.\\sqlexpress;Database=tacos;Trusted_Connection=True;TrustServerCertificate=True;MultipleActiveResultSets=true"
+ "DefaultConnection": "Server=.\\sqlexpress;Database=tacos;Trusted_Connection=True;MultipleActiveResultSets=true"
// src/tacos.mvc/appsettings.Development.json
{
  "ConnectionStrings": {
    "DefaultConnection": "Server=.\\sqlexpress;Database=tacos;Trusted_Connection=True;TrustServerCertificate=True;MultipleActiveResultSets=true"
  }
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"DefaultConnection": "Server=.\\sqlexpress;Database=tacos;Trusted_Connection=True;TrustServerCertificate=True;MultipleActiveResultSets=true"
"DefaultConnection": "Server=.\\sqlexpress;Database=tacos;Trusted_Connection=True;MultipleActiveResultSets=true"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/tacos.mvc/appsettings.json` at line 3, The base ConnectionStrings entry
"DefaultConnection" currently includes TrustServerCertificate=True; remove that
token from the base appsettings.json value so the production/default config
enforces strict TLS, and add a development-only override in
appsettings.Development.json (or an environment-specific override) that sets the
same "DefaultConnection" value but includes TrustServerCertificate=True for
local SQL Express testing; ensure the key name "DefaultConnection" is identical
between files so configuration fallback works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant