fix: re-plant the demo app's violations and guard them with a CI test - #48
fix: re-plant the demo app's violations and guard them with a CI test#48aanishs wants to merge 2 commits into
Conversation
The demo app exists to BE broken, but phase-0 commits remediated it in place (main.tf gained KMS/CloudTrail/flow logs, auth.ts gained bcrypt + session timeout) while the README violation table went stale — silently corrupting the benchmark target. This restores the planted violations from db27f21, moves the violation table out of the scanned tree (it is the answer key), and adds test/demo-integrity.test.ts so a future 'fix' to a planted violation fails CI instead of passing silently. Part of the Trust Sprint (issue #30 prep). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
em-dash Compliance Check
|
Greptile SummaryThis PR restores intentionally planted HIPAA violations in the demo app that had been silently remediated by earlier commits, and adds
Confidence Score: 4/5Safe to merge; the two findings are in non-production demo code and do not affect the tool's core behavior. The core idea — re-planting violations and locking them with CI assertions — is sound and well-executed across 11 of the 12 tests. The API_KEY assertion passes via accidental substring matching, so the guard's intent is unclear to anyone maintaining it later. The aws_lambda_function role reference is a broken Terraform attribute that is undetected by any guard; while the TF is never applied, it introduces an unexplained deviation from the demo's educational goal of showing realistic-but-insecure infrastructure. test/demo-integrity.test.ts (line 64 guard assertion) and demo/hipaa-demo-app/infra/main.tf (Lambda role attribute) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Developer modifies\ndemo/hipaa-demo-app] --> B{Modifies a\nplanted violation?}
B -- No --> C[PR merges normally]
B -- Yes --> D[test/demo-integrity.test.ts\nreads file from disk]
D --> E{Integrity assertion\npasses?}
E -- Yes\nViolation still present --> C
E -- No\nViolation was fixed --> F[CI fails ❌\nBlocks merge]
F --> G[Developer must update\nintegrity test in same PR\nmaking change visible & reviewable]
G --> C
subgraph Benchmark Protocol
H[Public planted set\nguarded by CI] --> I[em-dash demo scan]
J[Held-out set\npre-registered gist] --> I
I --> K[Blind detection score]
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[Developer modifies\ndemo/hipaa-demo-app] --> B{Modifies a\nplanted violation?}
B -- No --> C[PR merges normally]
B -- Yes --> D[test/demo-integrity.test.ts\nreads file from disk]
D --> E{Integrity assertion\npasses?}
E -- Yes\nViolation still present --> C
E -- No\nViolation was fixed --> F[CI fails ❌\nBlocks merge]
F --> G[Developer must update\nintegrity test in same PR\nmaking change visible & reviewable]
G --> C
subgraph Benchmark Protocol
H[Public planted set\nguarded by CI] --> I[em-dash demo scan]
J[Held-out set\npre-registered gist] --> I
I --> K[Blind detection score]
end
Reviews (1): Last reviewed commit: "fix: re-plant the demo app's violations ..." | Re-trigger Greptile |
| # VIOLATION: Security group with 0.0.0.0/0 — §164.312(a)(1) | ||
| resource "aws_security_group" "app" { | ||
| name = "hipaa-demo-app" |
There was a problem hiding this comment.
Lambda
role references a policy ARN, not a role ARN
aws_iam_policy.app_policy.arn resolves to a policy ARN (arn:aws:iam::…:policy/…), but Lambda's role attribute requires an IAM role ARN (arn:aws:iam::…:role/…). The old aws_iam_role.lambda_role resource was removed in this PR without a replacement. While this TF is never applied, it would fail at terraform apply with an InvalidParameterValueException, and the breakage is not an enumerated violation — undermining the file's value as a realistic (if insecure) reference.
| test(".env.example contains hardcoded secrets (§164.312(a)(1))", () => { | ||
| const env = read(".env.example"); | ||
| expect(env).toMatch(/AWS_SECRET_ACCESS_KEY=\S+/); | ||
| expect(env).toMatch(/API_KEY=\S+/); | ||
| }); |
There was a problem hiding this comment.
API_KEY guard matches wrong variable via substring
The assertion expect(env).toMatch(/API_KEY=\S+/) passes today because API_KEY= is a substring of ANTHROPIC_API_KEY=.... There is no standalone API_KEY variable in .env.example. If a future contributor reads this test and cross-references the file, they'll find no API_KEY= line — the guard silently shadows the actual variable name. The pattern should use ANTHROPIC_API_KEY=\S+ to unambiguously name the key being protected.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
…ble secret guards The wildcard-policy violation stays planted (now attached to a proper role); the Lambda role reference is a valid role ARN instead of a policy ARN. Secret guards anchor on exact variable names so API_KEY can't match via substring. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Both Greptile P2s addressed in the follow-up commit: the Lambda now references a real |
em-dash Compliance Check
|
Trust Sprint T3 (prep for #30 — the cold-run benchmark).
The problem: the demo app had been silently remediated in place —
main.tfgained KMS encryption, blocked public ACLs, CloudTrail, and VPC flow logs;auth.tsgained bcrypt and a session timeout — while the README's violation table went stale. The benchmark target for em-dash's detection claims was mostly fixed, and nothing caught it.This PR:
/comply-scancommand fixtest/demo-integrity.test.ts(12 assertions): any future 'fix' to a planted violation fails CI instead of passing silently. Enumerating the public set in-repo is by design — it's derivable from git history regardless; blind scoring uses a held-out set planted at run time and pre-registered via a timestamped public gistAll gates green locally: skill:check, biome, knip, and
bun run test(the 4 dashboard-api failures are the known local-env flake, green in CI).🤖 Generated with Claude Code