aws-samples / aws-samples/sample-autonomous-cloud-coding-agents

bug(bootstrap): BOOTSTRAP_HASH is blind to every policy action — JSON.stringify replacer misused as a key sort

Offen
#732 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
ci-cd security
Vorherrschende Sprache
TypeScript
Sterne
143
Forks
46
Ø Merge
3 T. 10 Std.
Gemergte PRs (30 T.)
24

Beschreibung

## Problem

`computeBootstrapHash()` (`cdk/src/bootstrap/version.ts:32`) is intended to be an integrity digest over the bootstrap policy bundle. It currently digests **nothing but statement counts** — every action, resource, condition and effect is invisible to it.

```ts
const normalized = policies.map((p) => {
const json = p.toJSON();
return JSON.stringify(json, Object.keys(json).sort()); // <-- bug
});
```

The second argument to `JSON.stringify` is a **replacer / property allowlist**, not a sort comparator. `Object.keys(json).sort()` evaluates to `['Statement', 'Version']`, so serialization is restricted to top-level properties with those names — and because `Statement` is an *array*, its element objects are filtered to `{}`.

The payload actually hashed:

```
[
"{\"Statement\":[{},{},{},{},{}],\"Version\":\"2012-10-17\"}",
"{\"Statement\":[{},{},{},{},{},{},{},{},{},{},{}],\"Version\":\"2012-10-17\"}",
"{\"Statement\":[{},{},{},{},{},{},{},{},{},{}],\"Version\":\"2012-10-17\"}",
"{\"Statement\":[{}],\"Version\":\"2012-10-17\"}",
"{\"Statement\":[{}],\"Version\":\"2012-10-17\"}"
]
```

Each policy serializes to 41–71 characters. The comment above the function ("policies are serialized with sorted keys so that object property ordering does not affect the digest") describes an intent the code does not implement.

## Impact

**The hash cannot detect a policy change.** Demonstrated while adding grants on #165: I added `s3:GetBucketPolicy`, `s3:GetEncryptionConfiguration`, `sqs:AddPermission`, `sqs:RemovePermission` to two policies, regenerated artifacts, and `BOOTSTRAP_HASH` was **byte-identical**:

```
committed : b0501c8a57f20e4b5bf50d6fe8bbb934310adc4b64480228565d8392a19d1503
computed : b0501c8a57f20e4b5bf50d6fe8bbb934310adc4b64480228565d8392a19d1503
match : true ← after adding 4 IAM actions
```

`artifact-sync.test.ts`'s "committed BOOTSTRAP_HASH matches computed hash" therefore passes vacuously. Anything that preserves statement *count* — swapping an action, widening a resource ARN from a named prefix to `*`, flipping `Effect` from `Deny` to `Allow` — leaves the digest untouched. For a construct whose stated purpose is bounding IaCRole blast radius (RFC #120), that is the wrong failure direction: a silent widening is exactly what it should catch.

Only the count is protected, so adding *or* removing a whole statement does move the hash. That is why this has gone unnoticed.

## Fix

Serialize deterministically over the full document instead of misusing the replacer. Either sort keys recursively:

```ts
const stableStringify = (v: unknown): string =>
Array.isArray(v) ? `[${v.map(stableStringify).join(',')}]`
: v && typeof v === 'object'
? `{${Object.keys(v as object).sort().map(k =>
`${JSON.stringify(k)}:${stableStringify((v as Record)[k])}`).join(',')}}`
: JSON.stringify(v) ?? 'null';
```

…or use the replacer as intended (a `(key, value)` function that sorts object keys). Note `iam.PolicyDocument.toJSON()` output is already key-ordered by the CDK, so plain `JSON.stringify(json)` may be sufficient — worth confirming before adding machinery.

Bump `BOOTSTRAP_VERSION` in the same change: the digest necessarily changes for every existing bundle, so this is a one-time re-baseline, not drift.

## Acceptance criteria

- Adding, removing, or **altering** any action / resource / effect in any bootstrap policy changes `BOOTSTRAP_HASH`.
- A regression test proves it: mutate one action in-memory and assert the digest differs. (Today an equivalent test would fail.)
- `artifact-sync.test.ts` still passes with regenerated artifacts.
- The function's doc comment matches what the code does.

## Provenance

Introduced by #122 ("policies as typed TypeScript with version and hash"), the step that added the hash. Not caused by #165 — verified the same code on `origin/main`. Surfaced on #165 only because that PR is the first to add IAM actions since, and the unchanged hash looked wrong.

Related: #120 (RFC), #124 (resource-action-map), #125/#126 (the Aspect and live validator that will rely on this digest being meaningful).

Beitragsleitfaden

Beitragsleitfaden öffnen

Rechercherichtung

Beginnen Sie bei computeBootstrapHash() in cdk/src/bootstrap/version.ts und untersuchen Sie, wie Policy-JSON serialisiert wird und wie BOOTSTRAP_VERSION definiert ist. Führen Sie artifact-sync.test.ts aus, fügen Sie die im Issue beschriebene Regressionstestabdeckung hinzu und verifizieren Sie, dass das Ändern einer Aktion den Digest ändert, während neu generierte Artefakte und die aktualisierte Dokumentation konsistent bleiben.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
aws, typescript
Bereich
authorization, security, testing
Issue-Typ
Bug
Schwierigkeit
3/5
Geschätzter Aufwand
1-2 Tage
Aktivitätsstatus
Ruhig
Klarheit
Klar beschrieben
Anfängerfreundlichkeit
72/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.