aws / aws/deep-learning-containers

ecr_scan.py: CVE gate queries scan findings by mutable tag while the resolved digest goes unused

Open
#6,517 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.