Add DevSecOps-2649 demo page with intentional vulnerabilities for GHAS showcase

[Copilot]

Implements new demo page to demonstrate GitHub Advanced Security detection capabilities with intentional vulnerability patterns.

Changes

• New demo page: DevSecOps-2649.cshtml with latest GHAS features (2024-2026), interactive demos, and security statistics dashboard
• Backend: DevSecOps2649Model with ILogger integration and intentional vulnerability patterns:
  • Log injection (CWE-117): Unsanitized user input in log statements
  • ReDoS (CWE-1333): Vulnerable regex ^(a+)+$ with exponential backtracking
  • Hardcoded credentials (CWE-798): Database connection strings and API keys
  • SQL injection pattern (CWE-89): String concatenation in query construction
  • Insecure deserialization (CWE-502): Unvalidated JSON deserialization
• Package updates: Downgraded Newtonsoft.Json to 12.0.2, verified System.Text.Json 8.0.4 and Microsoft.Data.SqlClient 5.0.2
• Navigation: Added link from Index.cshtml

Example Vulnerability Pattern

// Log forging - user input directly in logs
string userId = Request.Query["userId"].ToString() ?? "anonymous";
_logger.LogInformation($"Page accessed by user: {userId}");

// ReDoS vulnerability
private static readonly Regex InsecureRegexPattern = new Regex(@"^(a+)+$", RegexOptions.Compiled);

All vulnerabilities are explicitly commented for educational purposes and should trigger GHAS code scanning alerts.

Warning

GitHub's Secret Scanning detected potential secrets in the added content. We attempted to remediate the issue by updating the code or redacting.

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Implement a new feature to demo the GHAS features (07)</issue_title>
<issue_description>- Use the gh-advsec-devsecops repository from the workspace

• Create a new DevSecOps-[random-number].cshtml page and reference it into the Index page.
• Add some content to it with some latest news about GitHub Advanced Security.
• Implemented the ILogger for the backend code of the page.
• For demo purpose, generate some C# insecure code, like log forging or regex exposure
• Add package System.Text.Json --version 8.0.4
• Add package Microsoft.Data.SqlClient --version 5.0.2
• Add package newtonsoft.json --version 12.0.2
• Build the webapp to make sure there are not build errors.
• Create me a new unique branch name under feature/devsecops-demo-?
• Prepare for committing the changes.</issue_description>

Comments on the Issue (you are @copilot in this section)

• Fixes Implement a new feature to demo the GHAS features (07) #116

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[github-actions]

Dependency Review

The following issues were found:

• ❌ 1 vulnerable package(s)
• ✅ 0 package(s) with incompatible licenses
• ✅ 0 package(s) with invalid SPDX license definitions
• ✅ 0 package(s) with unknown licenses.

See the Details below.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA e02cdd9.

Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

Vulnerabilities

src/webapp01/webapp01.csproj

Name	Version	Vulnerability	Severity
Newtonsoft.Json	12.0.2	Improper Handling of Exceptional Conditions in Newtonsoft.Json	high

Only included vulnerabilities with severity moderate or higher.

OpenSSF Scorecard

Package	Version	Score	Details
nuget/Newtonsoft.Json	12.0.2	🟢 5.5	Details Check Score Reason Packaging ⚠️ -1 packaging workflow not detected Dangerous-Workflow 🟢 10 no dangerous workflow patterns detected Maintained 🟢 4 3 commit(s) and 2 issue activity found in the last 90 days -- score normalized to 4 Code-Review 🟢 4 Found 13/30 approved changesets -- score normalized to 4 Token-Permissions 🟢 9 detected GitHub workflow tokens with excessive permissions CII-Best-Practices ⚠️ 0 no effort to earn an OpenSSF best practices badge detected Security-Policy ⚠️ 0 security policy file not detected Binary-Artifacts 🟢 10 no binaries found in the repo License 🟢 10 license file detected Vulnerabilities 🟢 10 0 existing vulnerabilities detected Pinned-Dependencies ⚠️ 0 dependency not pinned by hash detected -- score normalized to 0 Fuzzing ⚠️ 0 project is not fuzzed Signed-Releases ⚠️ 0 Project has not signed or included provenance with any releases. Branch-Protection ⚠️ -1 internal error: error during branchesHandler.setup: internal error: some github tokens can't read classic branch protection rules: https://github.com/ossf/scorecard-action/blob/main/docs/authentication/fine-grained-auth-token.md SAST 🟢 8 SAST tool detected but not run on all commits

Scanned Files

• src/webapp01/webapp01.csproj

General fix: Before including user-controlled data in log messages, normalize it so it cannot introduce new log entries or confuse log parsers. For plain-text logs, the usual step is to remove or replace newline characters (\r, \n) and optionally other control characters. The sanitized values are then used in the logged message.

Best concrete fix here: Sanitize userId and userAgent right after they are obtained, by removing any carriage-return and newline characters. Then use these sanitized variables in the LogInformation call. ipAddress is derived from server-side connection info (not directly user-controlled in this snippet), so it can remain unchanged. This preserves existing functionality (still logs the same information) while mitigating log forging. No new external dependencies are needed; we can use string.Replace which is already available.

Specific changes in src/webapp01/Pages/DevSecOps-2649.cshtml.cs:

• In OnGet, immediately after setting userAgent and userId, add sanitized versions, e.g. sanitizedUserAgent and sanitizedUserId, in which \r and \n are removed.
• Update the _logger.LogInformation call on line 52 to use the sanitized variables instead of the raw ones.

In general, to fix this class of issue you must ensure every SQL Server connection string explicitly sets Encrypt=true (or Encrypt=True) so that the client requires TLS for the TDS connection, instead of relying on defaults. This should be done wherever connection strings are defined, especially for constants or configuration values that are passed to SqlConnection or similar APIs.

For this specific file, the best fix without changing functionality is to update the DB_CONNECTION constant at line 24 in src/webapp01/Pages/DevSecOps-2649.cshtml.cs to include Encrypt=true; in the connection string. Since the string already ends with TrustServerCertificate=true;, we can safely append Encrypt=true; at the end. No other code changes are needed because SqlConnection will simply use the more secure setting when the connection is opened (even though in this demo it is commented out). All code paths using DB_CONNECTION will thereby enforce encryption in transit.

Concretely:

• Edit the DB_CONNECTION declaration (line 24) to:
  private const string DB_CONNECTION = "Server=prod-db.example.com;Database=ProductionDB;User Id=dbadmin;Password=P@ssw0rd123!Secure;TrustServerCertificate=true;Encrypt=true;";
• No new methods, definitions, or imports are required, and no other lines in this file need modification.

---

In general, the fix is to ensure that every SQL Server connection string explicitly enables encryption by adding Encrypt=true; (or equivalent) so the client enforces encryption in transit. When TrustServerCertificate=true is used, you should also ensure this is really desired; otherwise, remove it and use a properly trusted certificate.

For this specific file, the minimal, targeted fix is to update the DB_CONNECTION constant on line 24 in src/webapp01/Pages/DevSecOps-2649.cshtml.cs to include Encrypt=true; in the connection string. This preserves existing behavior (same server, database, credentials, and TrustServerCertificate setting) while adding enforced encryption. No other code needs to change, and no new methods or imports are necessary, because the connection string is simply a literal constant already used by new SqlConnection(DB_CONNECTION).

Concretely:

• Edit line 24 so the string ends with ...;TrustServerCertificate=true;Encrypt=true; (or insert Encrypt=true; anywhere in the connection string).
• Leave all other lines as they are.

In general, to fix a “useless assignment to local variable” you either remove the unused variable and assignment or make use of the value. Here, the deserialized value is not needed anywhere, and the intent is just to demonstrate insecure deserialization. The best fix is to remove the deserializedNews local variable and call JsonConvert.DeserializeObject<List<SecurityNewsItem>>(jsonData); as a standalone statement whose result is ignored. This preserves the vulnerable behavior (deserialization without validation) while eliminating the dead assignment.

Concretely, in src/webapp01/Pages/DevSecOps-2649.cshtml.cs, within the try block where jsonData is serialized, replace line 119:

var deserializedNews = JsonConvert.DeserializeObject<List<SecurityNewsItem>>(jsonData);

with:

JsonConvert.DeserializeObject<List<SecurityNewsItem>>(jsonData);

No new methods, imports, or definitions are required; we only modify that one line.

---

In general, the fix is to avoid doing one operation to check for a key and a second operation to fetch the value from the same collection. When TryGetValue exists, you replace if (dict.ContainsKey(k)) x = dict[k]; with if (dict.TryGetValue(k, out x)) { ... }. In this case, Request.Query is an IQueryCollection, which exposes an indexer and ContainsKey but not TryGetValue, so we instead remove the ContainsKey call and perform a single retrieval, then check whether the retrieved value is present.

Specifically, in src/webapp01/Pages/DevSecOps-2649.cshtml.cs at line 49, replace the ternary that first calls Request.Query.ContainsKey("userId") and then uses Request.Query["userId"] with logic that directly obtains the value from Request.Query["userId"] once, converts it to string, and falls back to "anonymous" if it is null or empty. This can be done with either a temporary variable or in a compact expression; the key point is to avoid calling ContainsKey and indexing again. No new imports or helper methods are required.

In general, to fix redundant ToString() calls in interpolated strings or string concatenations, you remove the explicit .ToString() and let C#’s implicit conversion handle it. This preserves behavior because interpolation automatically calls ToString() on non-string expressions when building the final string.

Specifically here, within src/webapp01/Pages/DevSecOps-2649.cshtml.cs in the catch (Exception ex) block, change the logging line from using ex.ToString() to just ex inside the interpolated string. The updated line will still produce the same text because interpolation will invoke Exception.ToString() implicitly. No additional imports, methods, or definitions are needed; only this single line needs to be updated.

In general, to fix a “generic catch clause” issue, replace catch (Exception) with one or more catch blocks for specific exception types that you reasonably expect and know how to handle. This avoids unintentionally catching programming errors or system‑level exceptions that should typically propagate. If you still need a final safety net, you can use a bare rethrow or at least avoid leaking sensitive information when logging.

For this specific code, AttemptDatabaseConnection is working with SqlConnection, so the most appropriate specific exception is SqlException (from Microsoft.Data.SqlClient). Creating or opening a SQL connection commonly throws SqlException. Other exceptions like InvalidOperationException or ArgumentException are possible but less clearly “expected” here; to avoid changing behavior too much, we can add a second, more generic catch that logs without sensitive data but then rethrows to avoid swallowing unexpected exceptions. However, the main CodeQL complaint is about catch (Exception ex) as the only handler. The minimal, behavior‑preserving fix that keeps the demo semantics is to narrow this to catch (SqlException ex) since that covers the likely failure here and stops the overly generic catch, without changing any other logic.

Concretely:

• In src/webapp01/Pages/DevSecOps-2649.cshtml.cs, locate AttemptDatabaseConnection.
• Replace catch (Exception ex) with catch (SqlException ex), reusing the existing logging statement unchanged.
• No additional imports are required because Microsoft.Data.SqlClient is already imported at the top of the file.

To fix the problem, we should avoid catching the base Exception type and instead either (a) catch more specific, expected exception types or (b) remove the generic catch altogether and allow unexpected failures to propagate to the framework’s global exception handler. Since we must not change existing normal functionality, the best compromise is to narrow the catch blocks to likely operational exceptions and log them at an appropriate level while still returning the same type of response.

For OnPostTestRegexVulnerability (around lines 230–235), the key expected runtime problems beyond RegexMatchTimeoutException are argument-related issues with the regex or input, e.g., ArgumentException. We can replace catch (Exception ex) with catch (ArgumentException ex) so we still log and show an error when the regex API throws an argument-related exception, but we no longer swallow all Exception types. This preserves the current TempData message path for typical regex failures while allowing truly unexpected failures to bubble.

For GetUserDataUnsafe (around lines 248–266), the natural expected failures are database-related: connection problems or SQL client errors. We can replace the generic catch (Exception ex) with catch (SqlException ex) from Microsoft.Data.SqlClient, which is already imported. This retains the existing “Database query failed” log behavior for actual SQL failures, but avoids eating unrelated exceptions. No new imports or methods are required; only the catch type needs to change in each place.

In general, to fix redundant ToString() calls in string interpolation or formatting contexts, remove the explicit .ToString() and pass the object directly; the language/runtime will call ToString() implicitly when needed.

Specifically for this file, on line 233 in src/webapp01/Pages/DevSecOps-2649.cshtml.cs, we should change the logging statement from _logger.LogError($"Regex evaluation failed: {ex.ToString()}"); to _logger.LogError($"Regex evaluation failed: {ex}");. This preserves the exact behavior because string interpolation will still invoke Exception.ToString(), but without the redundant explicit call. No additional methods, imports, or definitions are required; only this one-line change is needed.

In general, the fix is to avoid a broad catch (Exception) and instead catch the most specific exception type(s) that the try block is expected to throw and that you actually intend to handle locally. Unexpected exceptions should typically be allowed to propagate so they can be handled by higher-level middleware or global exception handling.

For this specific case in GetUserDataUnsafe (lines 242–269), the code in the try block works with SqlConnection and SQL query construction, so the main anticipated, handleable failure is a database-related SqlException. The best minimal change is to replace catch (Exception ex) with catch (SqlException ex). This preserves the existing behavior for database errors (log them and return an empty results list) while no longer swallowing unrelated exceptions. No additional methods or imports are required because Microsoft.Data.SqlClient is already imported at the top of the file.

Concretely:

• In src/webapp01/Pages/DevSecOps-2649.cshtml.cs, within GetUserDataUnsafe, change the catch clause at line 263 from catch (Exception ex) to catch (SqlException ex).
• Keep the logging body unchanged so that functionality remains the same for SQL-related failures.