aws / aws/deep-learning-containers
ecr_scan.py: CVE gate queries scan findings by mutable tag while the resolved digest goes unused
- Dominant language
- Python
- Stars
- 1.2k
- Forks
- 559
- Avg merge
- 1d 7h
- Merged PRs (30d)
- 102
Description
## Summary
`test/security/scripts/ecr_scan.py` resolves the image digest and then never uses it. Every scan API call identifies the image by its **mutable tag**, so the CVE gate can report on a different image than the one the pipeline built.
## Environment
- Repo version: `main` @ [`a09e9fd`](https://github.com/aws/deep-learning-containers/commit/a09e9fdc74956fdf0ae87f01c788fc8936fa6ea0)
- Runs on the CI host via `_reusable.security-tests.yml`
- Python 3.12
## The defect
[`ecr_scan.py#L199-L216`](https://github.com/aws/deep-learning-containers/blob/a09e9fdc74956fdf0ae87f01c788fc8936fa6ea0/test/security/scripts/ecr_scan.py#L199-L216):
```python
img_resp = ecr_client.describe_images(
registryId=image.account_id,
repositoryName=image.repository,
imageIds=[{"imageTag": image.image_tag}],
)
sha = img_resp["imageDetails"][0]["imageDigest"]
LOGGER.info(f"Waiting for ECR enhanced scan: {image.repository}:{image.image_tag} ({sha})")
```
`sha` is resolved at L206 and used only in that log line. Both API callers still address the image by tag:
- `get_scan_status()` → `imageId={"imageTag": image.image_tag}` ([L53](https://github.com/aws/deep-learning-containers/blob/a09e9fdc74956fdf0ae87f01c788fc8936fa6ea0/test/security/scripts/ecr_scan.py#L53))
- `get_scan_findings()` → `image_id = {"imageTag": image.image_tag}` ([L63](https://github.com/aws/deep-learning-containers/blob/a09e9fdc74956fdf0ae87f01c788fc8936fa6ea0/test/security/scripts/ecr_scan.py#L63))
Resolving the digest and then only logging it reads as a half-finished intent — the author appears to have meant to pin the scan to a specific image.
## How to verify
Static, no AWS access needed:
```bash
git checkout a09e9fd
grep -n "imageTag\|imageDigest\|sha = " test/security/scripts/ecr_scan.py
```
Every `describe_image_scan_findings` call passes `imageTag`; `imageDigest` appears only on the line that reads it out of `describe_images`. `grep -rn "get_scan_status\|get_scan_findings"` confirms there are no callers outside this file.
## Impact
CI tags are deterministic and overwritten: `compute_ci_tag.sh` derives the CI tag from config metadata, so every rebuild of the same config pushes the **same tag** to a new digest. The script then reads findings across a long window in which the tag can move:
1. Pipeline A pushes `repo:ci-tag` → digest `sha256:AAA`, starts `ecr_scan.py`.
2. It polls for `imageScanStatus == COMPLETE` for up to 20 minutes (`SCAN_WAIT_PERIOD=40` × `SCAN_WAIT_LENGTH=30`), then sleeps a further `SCAN_POST_COMPLETE_WAIT=120` seconds before reading findings.
3. Inside that window a re-run, a dispatch build, or another caller pushes the same tag → digest `sha256:BBB`.
4. `describe_image_scan_findings` by tag now resolves to `sha256:BBB`.
The gate then reports on an image that is not the one under test. In the direction that matters for a security gate, an image with non-allowlisted CRITICAL/HIGH CVEs can pass because the findings returned belong to a different, cleaner digest.
The pipeline concurrency key includes `inputs.config-file`, which prevents two runs of the *same* config from racing, but nothing prevents a re-run, a dispatch build, or another caller from pushing the same CI tag while a scan is in flight.
## Suggested fix
Thread the already-resolved digest through `get_scan_status()` and `get_scan_findings()` as `imageId={"imageDigest": sha}` — ECR's `describe_image_scan_findings` accepts a digest in place of a tag. Keep the tag in log output for readability.
**Correction (edited):** this issue originally said no PR had been opened. I have since opened #6520 with the patch, so that line was no longer true and has been replaced. I am aware CONTRIBUTING.md asks external contributors not to open PRs — please close #6520 without review if that is the standing policy and treat this issue as the report. Patch also on my fork: https://github.com/Adityaj0/deep-learning-containers/pull/5
## Two adjacent observations
Independent of the above, both in `wait_for_status()` (`test/test_utils/__init__.py`):
- It sleeps *before* the first poll, so the first status check is always one `period_length` late — 30s here.
- It has no terminal-state check, so a scan reporting `FAILED` still polls for the full 20 minutes before giving up.
Happy to split these into their own issue if you would prefer.
Contributor guide
Research direction
Start in test/security/scripts/ecr_scan.py, reading get_scan_status(), get_scan_findings(), and the image-resolution code around lines 199-216. Use the supplied grep commands to verify how scan requests identify images; done means both scan calls consistently refer to the resolved digest while retaining the tag in logs.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- aws, python
- Domain
- cloud, security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 25/100