mirror of
https://github.com/Sea-Haven-Industries/proposal-system.git
synced 2026-09-30 11:13:14 +00:00
fix: wire test suites into CI, fix stale tests from Phase 1-2 fixes
- Add web-test job (vitest) and python-test job (pytest) to CI workflow - dotnet reusable workflow already runs tests by default - Update InternalApiKeyMiddleware tests for API-C1/API-H1 fixes: invalid key now returns 401 (not pass-through), valid key on disallowed path returns 403 - Fix suggestions test: include status field for LAM-H4 idempotency guard - Total: 108 tests (77 .NET, 12 web, 19 Python) all passing
This commit is contained in:
parent
9c04ba4756
commit
01fe003a6d
3 changed files with 72 additions and 24 deletions
37
.github/workflows/ci.yaml
vendored
37
.github/workflows/ci.yaml
vendored
|
|
@ -35,6 +35,23 @@ jobs:
|
|||
run-cdk-synth: false
|
||||
run-conventions-check: false
|
||||
|
||||
web-test:
|
||||
name: Web Tests
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 10
|
||||
defaults:
|
||||
run:
|
||||
working-directory: web
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
- uses: actions/setup-node@v4
|
||||
with:
|
||||
node-version: "24"
|
||||
cache: npm
|
||||
cache-dependency-path: web/package-lock.json
|
||||
- run: npm ci
|
||||
- run: npm test
|
||||
|
||||
python:
|
||||
name: Python Lint
|
||||
uses: Sea-Haven-Industries/.github/.github/workflows/ci-python-sam.yaml@main
|
||||
|
|
@ -43,6 +60,26 @@ jobs:
|
|||
run-sam-validate: false
|
||||
run-conventions-check: false
|
||||
|
||||
python-test:
|
||||
name: Python Tests
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 10
|
||||
defaults:
|
||||
run:
|
||||
working-directory: lambdas
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
- uses: actions/setup-python@v5
|
||||
with:
|
||||
python-version: "3.12"
|
||||
- name: Install dependencies
|
||||
run: |
|
||||
pip install -r tests/requirements-test.txt
|
||||
for req in $(find . -name requirements.txt -not -path './tests/*'); do
|
||||
pip install -r "$req"
|
||||
done
|
||||
- run: pytest tests/ -v
|
||||
|
||||
mobile:
|
||||
name: Mobile Typecheck
|
||||
uses: Sea-Haven-Industries/.github/.github/workflows/ci-typescript-cdk.yaml@main
|
||||
|
|
|
|||
|
|
@ -47,8 +47,8 @@ public class InternalApiKeyMiddlewareTests
|
|||
context.User.FindFirst(ClaimTypes.Email)!.Value.Should().Be("system@proposal-system.internal");
|
||||
}
|
||||
|
||||
[Fact(DisplayName = "QA-C4: Invalid key does not set claims but still calls next (falls through to JWT)")]
|
||||
public async Task InvalidKey_DoesNotSetClaims_CallsNext()
|
||||
[Fact(DisplayName = "QA-C4/API-H1: Invalid key returns 401 immediately")]
|
||||
public async Task InvalidKey_Returns401()
|
||||
{
|
||||
// Arrange
|
||||
var (middleware, context, nextCalled) = CreateMiddleware(ValidApiKey);
|
||||
|
|
@ -58,10 +58,9 @@ public class InternalApiKeyMiddlewareTests
|
|||
// Act
|
||||
await middleware.InvokeAsync(context);
|
||||
|
||||
// Assert
|
||||
nextCalled().Should().BeTrue("next middleware should still be called for JWT to handle");
|
||||
context.User.Identity!.IsAuthenticated.Should().BeFalse(
|
||||
"invalid key should not authenticate; JWT middleware handles auth next");
|
||||
// Assert — API-H1 fix: invalid key short-circuits with 401
|
||||
nextCalled().Should().BeFalse("invalid key should not fall through");
|
||||
context.Response.StatusCode.Should().Be(401);
|
||||
}
|
||||
|
||||
[Fact(DisplayName = "QA-C4: Missing key header passes through to next middleware")]
|
||||
|
|
@ -128,34 +127,26 @@ public class InternalApiKeyMiddlewareTests
|
|||
}
|
||||
|
||||
[Fact(DisplayName = "QA-C4: Timing-safe comparison used (key differs by one char)")]
|
||||
public async Task SimilarKey_DoesNotAuthenticate()
|
||||
public async Task SimilarKey_Returns401()
|
||||
{
|
||||
// This test verifies that a key differing by just one character
|
||||
// is still rejected (CryptographicOperations.FixedTimeEquals)
|
||||
var (middleware, context, nextCalled) = CreateMiddleware(ValidApiKey);
|
||||
context.Request.Headers["X-Internal-Api-Key"] = ValidApiKey + "x";
|
||||
context.Request.Path = "/api/proposals/123";
|
||||
|
||||
// Act
|
||||
await middleware.InvokeAsync(context);
|
||||
|
||||
// Assert
|
||||
nextCalled().Should().BeTrue();
|
||||
context.User.Identity!.IsAuthenticated.Should().BeFalse();
|
||||
// Assert — API-H1 fix: invalid key short-circuits with 401
|
||||
nextCalled().Should().BeFalse("similar but wrong key should not fall through");
|
||||
context.Response.StatusCode.Should().Be(401);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// API-C1 documents that the middleware currently authenticates on ANY path.
|
||||
/// This test verifies the current (vulnerable) behavior so that when
|
||||
/// path-scoping is added, this test can be updated to verify the fix.
|
||||
/// </summary>
|
||||
[Fact(DisplayName = "QA-C4/API-C1: Valid key currently authenticates on any path (documents vulnerability)")]
|
||||
public async Task ValidKey_AuthenticatesOnAnyPath_DocumentsApiC1()
|
||||
[Fact(DisplayName = "QA-C4/API-C1: Valid key on allowed path authenticates")]
|
||||
public async Task ValidKey_AllowedPath_Authenticates()
|
||||
{
|
||||
// The middleware does not scope to /internal/ paths — API-C1 finding.
|
||||
// This test documents the current behavior.
|
||||
var paths = new[] { "/api/proposals", "/api/admin/dashboard", "/api/users", "/health" };
|
||||
var allowedPaths = new[] { "/api/proposals", "/api/vendor-proposals/1", "/api/generated-pdfs", "/api/files/upload" };
|
||||
|
||||
foreach (var path in paths)
|
||||
foreach (var path in allowedPaths)
|
||||
{
|
||||
var (middleware, context, nextCalled) = CreateMiddleware(ValidApiKey);
|
||||
context.Request.Headers["X-Internal-Api-Key"] = ValidApiKey;
|
||||
|
|
@ -164,7 +155,26 @@ public class InternalApiKeyMiddlewareTests
|
|||
await middleware.InvokeAsync(context);
|
||||
|
||||
context.User.Identity!.IsAuthenticated.Should().BeTrue(
|
||||
$"API-C1: middleware currently authenticates on {path} (should be scoped to /internal/ paths)");
|
||||
$"valid key should authenticate on allowed path {path}");
|
||||
nextCalled().Should().BeTrue();
|
||||
}
|
||||
}
|
||||
|
||||
[Fact(DisplayName = "QA-C4/API-C1: Valid key on disallowed path returns 403")]
|
||||
public async Task ValidKey_DisallowedPath_Returns403()
|
||||
{
|
||||
var disallowedPaths = new[] { "/api/admin/dashboard", "/api/users", "/health" };
|
||||
|
||||
foreach (var path in disallowedPaths)
|
||||
{
|
||||
var (middleware, context, nextCalled) = CreateMiddleware(ValidApiKey);
|
||||
context.Request.Headers["X-Internal-Api-Key"] = ValidApiKey;
|
||||
context.Request.Path = path;
|
||||
|
||||
await middleware.InvokeAsync(context);
|
||||
|
||||
nextCalled().Should().BeFalse($"valid key on disallowed path {path} should not call next");
|
||||
context.Response.StatusCode.Should().Be(403);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -100,6 +100,7 @@ class TestProcessSuggestion:
|
|||
"scopeOfWork": "Replace HVAC system",
|
||||
"serviceCategory": "HVAC",
|
||||
"priority": "Standard",
|
||||
"status": "InReview",
|
||||
}
|
||||
mock_fetch_items.return_value = []
|
||||
mock_retrieve.return_value = [{"content": "similar doc", "score": 0.9, "metadata": {}, "sourceUri": ""}]
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue