When Remediation Creates New Findings

One Monday afternoon I committed the remediation for a pre-production security review of a serverless API: three critical findings, six high, nine medium, ten low. The commit closed twenty-seven of them and retired the twenty-eighth. Its message ends with the line I had been trained to treat as the finish: 86 tests passing (57 unit + 29 contract).

Thirty-seven minutes later I committed again. That commit is titled resolve all re-audit findings (REGRESSION-1, NEW-1 through NEW-15), and its message ends with exactly the same line: 86 tests passing (57 unit + 29 contract). Thirty-seven minutes had produced one critical regression and fifteen new findings, and the test suite could not tell the two states apart. That is the normal output of a remediation batch, not a sign of a bad first pass. The unusual part was looking.

A remediation batch is a change, and changes carry defects

The first review covered the codebase, the infrastructure template, the delivery pipelines and the dependency tree, and returned 28 findings: 3 critical, 6 high, 9 medium, 10 low. Fixing twenty-seven of them took one commit across 33 files, 956 lines added and 619 removed, of which 241 added lines were in the infrastructure template alone. That is a feature, not a patch, and it was written fast under the specific pressure of wanting a clean board.

Every argument for reviewing a feature branch applies to that change set with more force, because remediation is written by someone who already believes they know what correct looks like. Yet the standard practice is to close findings on the strength of the diff that claims to close them. The reviewer who found the problem is rarely the person who reads the fix, and the fix is rarely read at all.

Sixteen findings in thirty-seven minutes is not a failure of the first review. It is the yield you should expect from re-reviewing 241 lines of new infrastructure code that nobody has looked at yet, because that is precisely what a remediation branch is.

A control added is not a control enforcing

Finding H4 read "No WAF on Public API Gateway." The remediation added a web application firewall as an AWS::WAFv2::WebACL resource, carrying the two managed rule groups the review had asked for, and the finding was marked resolved:

# before (rules abridged)
WebACL:
  Type: AWS::WAFv2::WebACL
  Properties:
    Name: !Sub "axesoffate-waf-${Stage}"
    Scope: REGIONAL
    Rules:
      - Name: AWSManagedRulesCommonRuleSet
      - Name: AWSManagedRulesKnownBadInputsRuleSet

That web access control list inspected nothing. There is no AWS::WAFv2::WebACLAssociation in the template, and there could not have been one: the API is an AWS::Serverless::HttpApi, and AWS WAF's documented list of protectable resources names "Amazon API Gateway REST API" and does not include HTTP APIs. A regional web access control list with no association is a billable object with a rule set and no traffic.

The re-audit filed this as REGRESSION-1, critical. The fix was not a two-line change:

# after (abridged)
WebACL:
  Properties:
    Scope: CLOUDFRONT

CloudFrontDistribution:
  Type: AWS::CloudFront::Distribution
  Properties:
    DistributionConfig:
      WebACLId: !GetAtt WebACL.Arn
      Origins:
        - DomainName: !Sub "${HttpApi}.execute-api.${AWS::Region}.amazonaws.com"

Attaching the firewall meant putting a content delivery network in front of the API, which changed the public URL the mobile client had to call, and moved the firewall into a different region, because CloudFormation's reference for that resource states that for CLOUDFRONT scope "you must create your WAFv2 resources in the US East (N. Virginia) Region, us-east-1." A finding that looked like one missing resource turned into a topology change.

The general shape is worth naming. Attachment blindness. A diff shows the resources you added; it cannot show an association you failed to add, because absence has no line number. Every reviewer reading that first commit would have seen a firewall with correct managed rules and moved on. The only question that catches it is asked about the deployed system rather than the source: what resource is this control bound to, and what would show me the binding?

Three re-audit fixes did not survive first deploy

That second commit added three infrastructure controls: a reserved Lambda concurrency of 50, flow logs on the virtual network, and the content delivery distribution carrying the firewall. All three were reasonable. All three were gone within a hundred minutes.

The distribution came out within the hour, because it referenced the API identifier and produced a circular dependency during initial stack creation. The flow logs came out half an hour after that, with the commit noting they "require a DeliverLogsPermissionArn" and deferring rather than adding another CloudWatch Logs delivery role to the first deploy. The concurrency reservation came out seven minutes later: "Account Lambda concurrency limit doesn't support reserving 50." The firewall came back that evening, as a separate stack deployed in us-east-1. The other two did not. The template still carries this line at the last commit on the branch:

# [NEW-12] VPC Flow Logs — deferred to post-deploy (requires IAM role for CW Logs delivery)

Both are recorded as resolved in the pre-production checklist, and neither is in the template. That is the same defect as the unattached firewall wearing different clothes: a control that exists as an intention in a document and as nothing in the account.

What I changed as a result is narrow and cheap. A finding is not closed by a commit. It is closed by an observation of the running system, and until that observation exists the correct status is "fix written," which is a different column.

Write down what you refused to fix

The re-audit also produced findings I chose not to fix, and those went into a decisions log with the alternatives spelled out. The entry for the single network address translation gateway is one line of decision, one line of context and three numbered alternatives, ending with a trigger rather than a date: "Revisit when: User base grows beyond hobby scale ..."

The value of that file showed up in an argument it settled with itself. The first commit, which closed twenty-seven findings, declined the twenty-eighth: L10, a low-severity note about the API having no custom domain, moved into the decisions log with its alternatives written out. Alternative 3 in that entry reads: "CloudFront with default domain — Provides a stable d1234.cloudfront.net URL ... Could add later if needed." REGRESSION-1 made alternative 3 mandatory. Because the log had recorded reasoning rather than a verdict, the later entry could adopt the option the earlier one had passed over without either being wrong. A decision log that stores only outcomes cannot do that.

The same commit also edited the review document's own summary table from ten lows to nine, and struck L10's heading through in place. The count that says twenty-seven was written by the commit that closed twenty-seven.

The strongest objection

The strongest objection to budgeting a re-audit is that it does not terminate. If a remediation batch is a change and changes carry defects, then the re-audit's fixes are also a change, and they need a third pass, and so on. My own timeline is evidence for the objection rather than against it: three of the sixteen re-audit fixes could not deploy as written, and no fourth reviewer found that out.

That is right, and it is why the loop closes somewhere other than review. What caught the circular dependency, the missing log delivery role and the account concurrency ceiling was not a reviewer. It was the deploy: three commits inside an hour, each one removing a control and naming the constraint that had stopped the stack from creating. Deployment is a better second reviewer than a second reviewer, for everything expressible as a resource. The reason to spend a human pass in between is the class of defect deployment is blind to, and the unattached firewall is exactly that class: it deployed cleanly, cost money, reported metrics, and inspected no requests. So the budget I now argue for is one re-audit, scoped to the remediation diff, followed by a deploy that has to succeed before any finding changes status.

What to check before you close a remediation batch

If you are about to mark a set of findings resolved, this is the pass that costs an hour:

1. Re-run the review against the remediation diff, not against the original commit.
2. For each control added, name the resource it protects and the object that binds the two.
3. Query that binding from outside the template: aws wafv2 list-resources-for-web-acl for regional scope, ListDistributionsByWebACLId for CloudFront scope. Needing two different calls is part of why nobody checks.
4. Deploy to a real account before closing anything; a template that will not deploy has closed nothing.
5. Diff the deployed template against the one the review approved, and treat every deletion as a reopened finding.
6. Confirm each pipeline gate you added actually gates, by pushing a change that should fail it.
7. Record every finding you deliberately did not fix, with its alternatives and a revisit trigger.

A bad result on step five looks like mine: three resources present in the reviewed template and absent from the deployed one, all three still ticked in the checklist.

The rule I take from this: a control has two states that a diff renders identically, added and enforcing, so no finding closes on a commit and every finding closes on a query against the running system. It holds wherever the control is a resource with a binding you can interrogate. It does not help with controls whose enforcement is a code path, where the equivalent question is which request would prove it, and I do not yet have a clean way to make that one routine.

Both of those commit messages ended with 86 tests passing (57 unit + 29 contract). The suite was right both times. It was simply answering a question nobody had asked about a firewall.