Fix CodeQL security vulnerabilities in CORS, JWT, logging, and GitHub Actions

[Copilot]

CodeQL identified five security vulnerabilities across authentication, logging, and deployment workflows.

Changes

• CORS policy: Removed wildcard "*" from allowed origins that permitted any site to make authenticated requests
• JWT configuration: RequireHttpsMetadata now respects environment—disabled only in development
• Logging: Replaced Console.WriteLine with ILogger<FileImportService> to prevent information disclosure
• Exception handling: Added specific exception types and logging instead of empty catch blocks
• GitHub Actions: Fixed token exposure by piping to stdin instead of echoing in remote shell

Example

Before:

public class FileImportService
{
    // ...
    catch
    {
        failedCount++;
    }
    Console.WriteLine($"Processed {records.Count + failedCount} / {parsedData.Rows.Count}, Failed: {failedCount}");
}

After:

public class FileImportService(ILogger<FileImportService> logger)
{
    private readonly ILogger<FileImportService> _logger = logger;
    // ...
    catch (Exception ex)
    {
        failedCount++;
        _logger.LogWarning(ex, "Failed to parse row {RowIndex} during import", records.Count + failedCount);
    }
    _logger.LogDebug("Processed {ProcessedRows} / {TotalRows}, Failed: {FailedRows}", 
        records.Count + failedCount, parsedData.Rows.Count, failedCount);
}

CodeQL analysis: 0 alerts remaining.

Original prompt

Can you check the CodeQL reports and fix those?

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[Copilot]

Pull Request Overview

This PR addresses five CodeQL security vulnerabilities related to authentication, logging, and deployment workflows. The changes remove the wildcard CORS origin, make JWT HTTPS validation environment-aware, replace console logging with structured logging, add specific exception handling, and fix token exposure in GitHub Actions.

Key changes:

• Removed wildcard "*" from CORS allowed origins
• Made JWT RequireHttpsMetadata environment-dependent (enabled in production)
• Replaced Console.WriteLine with ILogger<FileImportService> for secure, structured logging

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
PhantomDave.BankTracking.Api/Services/FileImportService.cs	Injected ILogger<FileImportService>, replaced Console.WriteLine with structured logging, added specific exception types (DecoderFallbackException, Exception) instead of empty catch blocks
PhantomDave.BankTracking.Api/Program.cs	Removed wildcard "*" from CORS origins, made RequireHttpsMetadata conditional on environment (false in dev, true in production)
.github/workflows/deploy-tailscale.yml	Fixed token exposure by piping GITHUB_TOKEN to stdin locally instead of echoing in remote shell

Comments suppressed due to low confidence (7)

PhantomDave.BankTracking.Api/Program.cs:88

• The CORS policy is hardcoded to localhost origins and will apply to all environments including production. In production, this will block legitimate requests from the actual production frontend URL. Consider making CORS origins configurable via appsettings.json or environment variables, and using builder.Environment.IsDevelopment() to conditionally allow localhost origins only in development:

var allowedOrigins = builder.Configuration.GetSection("Cors:AllowedOrigins").Get<string[]>() 
    ?? (builder.Environment.IsDevelopment() 
        ? new[] { "http://localhost:4200", "http://localhost:5095" }
        : Array.Empty<string>());

        builder.Services.AddCors(options =>
        {
            options.AddDefaultPolicy(policy =>
                policy
                    .WithOrigins("http://localhost:4200", "http://localhost:5095")
                    .AllowAnyHeader()
                    .AllowAnyMethod()
                    .AllowCredentials());
        });

PhantomDave.BankTracking.Api/Services/FileImportService.cs:176

• This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.

                    foreach (var keyword in headerKeywords)
                    {
                        if (lowerText.Contains(keyword))
                        {
                            score += 10;
                            break;
                        }
                    }

PhantomDave.BankTracking.Api/Services/FileImportService.cs:263

• Inefficient use of 'ContainsKey' and indexer.

                if (input.ColumnMappings.TryGetValue("Date", out var dateColumn) && row.ContainsKey(dateColumn))

PhantomDave.BankTracking.Api/Services/FileImportService.cs:272

• Inefficient use of 'ContainsKey' and indexer.

                if (input.ColumnMappings.TryGetValue("Amount", out var amountColumn) && row.ContainsKey(amountColumn))

PhantomDave.BankTracking.Api/Services/FileImportService.cs:280

• Inefficient use of 'ContainsKey' and indexer.

                if (input.ColumnMappings.TryGetValue("Description", out var descriptionColumn) && row.ContainsKey(descriptionColumn))

PhantomDave.BankTracking.Api/Services/FileImportService.cs:270

• These 'if' statements can be combined.

                if (input.ColumnMappings.TryGetValue("Date", out var dateColumn) && row.ContainsKey(dateColumn))
                {
                    if (DateTime.TryParseExact(row[dateColumn], input.DateFormat, CultureInfo.InvariantCulture, DateTimeStyles.None, out var date))
                    {
                        // Ensure UTC kind to satisfy Npgsql 'timestamp with time zone' requirement
                        record.Date = DateTime.SpecifyKind(date, DateTimeKind.Utc);
                    }
                }

PhantomDave.BankTracking.Api/Services/FileImportService.cs:278

• These 'if' statements can be combined.

                if (input.ColumnMappings.TryGetValue("Amount", out var amountColumn) && row.ContainsKey(amountColumn))
                {
                    if (decimal.TryParse(row[amountColumn], NumberStyles.Any, CultureInfo.InvariantCulture, out var amount))
                    {
                        record.Amount = amount;
                    }
                }

[Copilot]

The debug log is being written inside the loop for every row, which could generate excessive log output. This should be moved outside the foreach loop to log the final count only once after all rows have been processed.

Suggested change

	_logger.LogDebug("Processed {ProcessedRows} / {TotalRows}, Failed: {FailedRows}",
	records.Count + failedCount, parsedData.Rows.Count, failedCount);
	}
	}
	_logger.LogDebug("Processed {ProcessedRows} / {TotalRows}, Failed: {FailedRows}",
	records.Count + failedCount, parsedData.Rows.Count, failedCount);

[Copilot]

Encoding.UTF8.GetString by default uses DecoderFallback.ReplacementFallback, which replaces invalid bytes with '�' instead of throwing DecoderFallbackException. The catch block will never be reached. To properly detect UTF-8, you need to create a UTF-8 decoder with DecoderFallback.ExceptionFallback:

var utf8 = Encoding.GetEncoding("UTF-8", new EncoderExceptionFallback(), new DecoderExceptionFallback());
utf8.GetString(buffer, 0, bytesRead);

Suggested change

	Encoding.UTF8.GetString(buffer, 0, bytesRead);
	var strictUtf8 = Encoding.GetEncoding("UTF-8", new EncoderExceptionFallback(), new DecoderExceptionFallback());
	strictUtf8.GetString(buffer, 0, bytesRead);

[Copilot]

The comment says "Set to false only for local development" but the code builder.Environment.IsDevelopment() == false evaluates to true in production (not development) and false in development. Consider rephrasing the comment to be clearer:

// RequireHttpsMetadata is disabled in development and enabled in production

Suggested change

	// WARNING: RequireHttpsMetadata should be true in production environments
	// Set to false only for local development
	// RequireHttpsMetadata is disabled in development and enabled in production
	// (i.e., set to false in development, true in production)

[Copilot]

The expression 'A == false' can be simplified to '!A'.

Suggested change

	options.RequireHttpsMetadata = builder.Environment.IsDevelopment() == false;
	options.RequireHttpsMetadata = !builder.Environment.IsDevelopment();

[Copilot]

Generic catch clause.

Suggested change

	catch (Exception ex)
	{
	failedCount++;
	_logger.LogWarning(ex, "Failed to parse row {RowIndex} during import", records.Count + failedCount);
	}
	catch (FormatException ex)
	{
	failedCount++;
	_logger.LogWarning(ex, "Failed to parse row {RowIndex} due to format error during import", records.Count + failedCount);
	}
	catch (KeyNotFoundException ex)
	{
	failedCount++;
	_logger.LogWarning(ex, "Failed to parse row {RowIndex} due to missing column during import", records.Count + failedCount);
	}
	catch (InvalidCastException ex)
	{
	failedCount++;
	_logger.LogWarning(ex, "Failed to parse row {RowIndex} due to invalid cast during import", records.Count + failedCount);
	}