diff --git a/scripts/dead-weight.py b/scripts/dead-weight.py index 20f10f9..917a10e 100755 --- a/scripts/dead-weight.py +++ b/scripts/dead-weight.py @@ -1106,8 +1106,8 @@ def _stage_gpu_map(text: str): installs into the CPU runtime, and that install is exactly the finding (Bugbot, .github#454). A stage inherits GPU-ness from an earlier stage it is `FROM ` of; anything before the first FROM (ARGs) is no stage.""" - stages = [] # (start_line, gpu) - named = {} + stages = [] # (start_line, gpu, unresolved) + named = {} # alias -> (gpu, unresolved) args, seen_from = {}, False for no, raw in enumerate(text.splitlines(), 1): am = ARG_LINE.match(raw.split(" #", 1)[0]) @@ -1121,22 +1121,41 @@ def _stage_gpu_map(text: str): ref, _ = _expand_args(m.group(1), args) # `FROM ${CUDA_IMAGE}` is judged by what it expands to (Bugbot, .github#454) alias = re.search(r"\s(?:AS|as)\s+(\S+)\s*(?:#.*)?$", raw) if ref in named: - gpu = named[ref] + gpu, unresolved = named[ref] else: - gpu = bool(GPU_HINT.search(_image_name_tag(ref)[0] + " " + ref)) + # Judge GPU-ness on the RESOLVED text only: an unresolved `${ARG}` + # placeholder must not satisfy GPU_HINT through its own name -- an + # unset `FROM ${CUDA_IMAGE}` would otherwise read as a GPU stage and + # silently clear a CPU-torch finding without knowing the image, the + # very case check_full_python_base reports as cannot-parse + # (Bugbot, .github#457). + resolved = ARG_REF.sub("", ref) + name = _image_name_tag(resolved)[0] + gpu = bool(GPU_HINT.search(name + " " + resolved)) + # The image is unknown only when expansion leaves no image NAME at + # all -- an unset `${CUDA_IMAGE}`/`${BASE}`, or a nested default like + # `${IMAGE:-${GPU_BASE}}` that `_expand_args` does not recurse into, + # whose inner ref the strip above drops (Bugbot, .github#459). A name + # templated only in its registry or tag (`${REGISTRY}/python:3.11-slim`, + # `nvidia/cuda:${TAG}`) is known -- judge it, never cannot-parse. + unresolved = not name and not gpu if alias: - named[alias.group(1)] = gpu - stages.append((no, gpu)) + named[alias.group(1)] = (gpu, unresolved) + stages.append((no, gpu, unresolved)) - def lookup(line_no: int) -> bool: - current = False - for start, gpu in stages: + def _at(line_no: int, idx: int) -> bool: + current = (False, False) + for start, gpu, unres in stages: if start <= line_no: - current = gpu + current = (gpu, unres) else: break - return current + return current[idx] + + def lookup(line_no: int) -> bool: + return _at(line_no, 0) + lookup.unresolved_at = lambda line_no: _at(line_no, 1) lookup.stages = stages return lookup @@ -1146,7 +1165,7 @@ def _stage_text(text: str, stages, line_no: int) -> str: does not survive a FROM, so a CPU index set in another stage -- before or after -- says nothing about this stage's install (Bugbot, .github#454).""" lines = text.splitlines() - starts = [start for start, _ in stages] + starts = [start for start, *_ in stages] begin = max([st for st in starts if st <= line_no], default=1) later = [st for st in starts if st > line_no] end = min(later) - 1 if later else len(lines) @@ -1154,8 +1173,11 @@ def _stage_text(text: str, stages, line_no: int) -> str: def _installers_of(repo: Repo, req_basename: str, want_file_rel: str): - """(rel, line, command, gpu_context) for every Dockerfile RUN / workflow step - that pip-installs a requirements file with this basename.""" + """(rel, line, command, gpu_context, context_text, unresolved) for every + Dockerfile RUN / workflow step that pip-installs a requirements file with + this basename. `unresolved` is true only when the install sits in a stage + whose base image is an ARG with no value, so GPU-or-CPU cannot be told + (Bugbot, .github#457).""" hits = [] for rel in repo.glob("Dockerfile*", "*.Dockerfile", "*.dockerfile"): text = DOCKER_COMMENT.sub("", repo.text(rel)) # a commented-out RUN installs nothing (Bugbot, .github#454) @@ -1166,7 +1188,11 @@ def _installers_of(repo: Repo, req_basename: str, want_file_rel: str): continue for target in REQ_FLAG.findall(cmd): if os.path.basename(target) == req_basename: - hits.append((rel, no, cmd, file_gpu or stage_gpu(no), _stage_text(text, stage_gpu.stages, no))) + gpu = file_gpu or stage_gpu(no) + # unresolved only matters when GPU-ness is otherwise unknown: + # a GPU filename or a resolved GPU stage already settles it. + unresolved = not gpu and stage_gpu.unresolved_at(no) + hits.append((rel, no, cmd, gpu, _stage_text(text, stage_gpu.stages, no), unresolved)) for rel in repo.glob(".github/workflows/*.yml", ".github/workflows/*.yaml"): for job_start, job_text, context in _workflow_jobs(repo.text(rel)): gpu = any(GPU_HINT.search(m.group(1)) for m in RUNS_ON.finditer(job_text)) @@ -1175,7 +1201,7 @@ def _installers_of(repo: Repo, req_basename: str, want_file_rel: str): continue for target in REQ_FLAG.findall(cmd): if os.path.basename(target) == req_basename: - hits.append((rel, job_start + offset - 1, cmd, gpu, context)) + hits.append((rel, job_start + offset - 1, cmd, gpu, context, False)) return hits @@ -1255,11 +1281,22 @@ def check_cuda_torch_on_cpu(repo: Repo, cfg: Config, findings): installers = [] for b in basenames: installers.extend(_installers_of(repo, b, rel)) - for irel, ino, cmd, gpu, itext in installers: + for irel, ino, cmd, gpu, itext, unresolved in installers: if gpu: continue if CPU_INDEX.search(cmd) or re.search(r"PIP_(?:EXTRA_)?INDEX_URL[^\n]*whl/cpu", itext): continue + if unresolved: + # The stage's base image comes from an ARG with no value in this + # file, so GPU-or-CPU cannot be told and a CUDA-on-CPU torch + # install can be neither confirmed nor ruled out. Scan integrity, + # hard in every mode -- matching check_full_python_base, not a + # silent clean pass (Bugbot, .github#457). + findings.append(Finding( + "cannot-parse", irel, ino, + "torch is installed here in a stage whose base image comes from an ARG with no value in this file, " + "so GPU-or-CPU cannot be told; give the ARG a default (else a CUDA-on-CPU torch install cannot be caught)")) + continue for pin in pins: findings.append(Finding( "cuda-torch-on-cpu", pin.path, pin.line, diff --git a/scripts/tests/dead-weight-mutations.py b/scripts/tests/dead-weight-mutations.py index b562e10..07d478b 100644 --- a/scripts/tests/dead-weight-mutations.py +++ b/scripts/tests/dead-weight-mutations.py @@ -101,8 +101,8 @@ " if comment:"), ("an unresolved ARG tag is silently accepted", - " if unresolved:", - " if False:"), + " if unresolved:\n # Cannot tell slim from full", + " if False:\n # Cannot tell slim from full"), ("torch pins are never judged", ' if "+cpu" in raw:\n continue', @@ -196,8 +196,8 @@ " integrity = []\n if integrity:"), ("a GPU stage anywhere in a Dockerfile exempts every stage", - " hits.append((rel, no, cmd, file_gpu or stage_gpu(no), _stage_text(text, stage_gpu.stages, no)))", - " hits.append((rel, no, cmd, file_gpu or any(stage_gpu(n) for n in range(1, no + 1)), _stage_text(text, stage_gpu.stages, no)))"), + " gpu = file_gpu or stage_gpu(no)", + " gpu = file_gpu or any(stage_gpu(n) for n in range(1, no + 1))"), ("eslint evidence reads the raw package.json again (dependency keys count as extends)", ' chunks = [repo.text(rel) for rel in repo.glob(*ESLINT_CONFIG_GLOBS)]', @@ -269,11 +269,23 @@ ("a GPU base behind a FROM ARG reads as CPU", r''' ref, _ = _expand_args(m.group(1), args) # `FROM ${CUDA_IMAGE}` is judged by what it expands to (Bugbot, .github#454)''', - r''' ref = m.group(1)'''), + r''' ref, _ = m.group(1), False'''), + + ("an unresolved FROM ARG's own name marks the stage GPU (`${CUDA_IMAGE}` reads GPU)", + r''' resolved = ARG_REF.sub("", ref)''', + r''' resolved = ref'''), + + ("an install in an unresolved-base stage is silently cleared, not cannot-parse", + " if unresolved:\n # The stage's base image comes from an ARG with no value in this", + " if False:\n # The stage's base image comes from an ARG with no value in this"), + + ("the resolved NAME, not the raw ref, decides an unknown image (nested-default / empty-name)", + " unresolved = not name and not gpu", + " unresolved = not ref and not gpu"), ("a CPU index anywhere in the Dockerfile clears every stage's install", - r''' hits.append((rel, no, cmd, file_gpu or stage_gpu(no), _stage_text(text, stage_gpu.stages, no)))''', - r''' hits.append((rel, no, cmd, file_gpu or stage_gpu(no), text))'''), + r''' hits.append((rel, no, cmd, gpu, _stage_text(text, stage_gpu.stages, no), unresolved))''', + r''' hits.append((rel, no, cmd, gpu, text, unresolved))'''), ("trailing Dockerfile comments are kept (hide exec-form installs, name indexes)", r'''DOCKER_COMMENT = re.compile(r"^\s*#.*$|\s#[^\"'\n]*$", re.M)''', diff --git a/scripts/tests/dead-weight-selftest.py b/scripts/tests/dead-weight-selftest.py index 99a8996..9ddf47f 100644 --- a/scripts/tests/dead-weight-selftest.py +++ b/scripts/tests/dead-weight-selftest.py @@ -633,15 +633,61 @@ def _(): assert_clean(fx.findings(["cuda-torch-on-cpu"])) -@case("cuda-torch: a GPU base behind a pre-FROM ARG (`FROM ${BASE_IMAGE}`) is a GPU stage") +@case("cuda-torch: a GPU base behind a pre-FROM ARG (`FROM ${BASE_IMAGE}`) is a GPU stage; a CPU one is a finding") def _(): # the ARG name carries no gpu word on purpose: only the EXPANDED value can say GPU fx = Fixture({"requirements.txt": REQ_TORCH, "Dockerfile": "ARG BASE_IMAGE=nvidia/cuda:12.4.1-runtime-ubuntu22.04\nFROM ${BASE_IMAGE}\nRUN pip install -r requirements.txt\n"}) assert_clean(fx.findings(["cuda-torch-on-cpu"])) + # the CPU counterpart: a resolved slim image via ARG is a normal finding, not cannot-parse cpu_arg = Fixture({"requirements.txt": REQ_TORCH, "Dockerfile": "ARG BASE=python:3.11-slim\nFROM ${BASE}\nRUN pip install -r requirements.txt\n"}) assert_finding(cpu_arg.findings(["cuda-torch-on-cpu"]), "cuda-torch-on-cpu", count=1) +@case("cuda-torch: an unset base ARG is cannot-parse, never a clean pass -- its own name (`${CUDA_IMAGE}`) must not read as GPU") +def _(): + # The ARG name contains `cuda`; before the fix the unexpanded `${CUDA_IMAGE}` + # satisfied GPU_HINT and the CPU-torch install was silently cleared without + # knowing the image (Bugbot, .github#457). It is now scan integrity, like + # check_full_python_base treats the same ambiguity. + cuda_named = Fixture({"requirements.txt": REQ_TORCH, "Dockerfile": "ARG CUDA_IMAGE\nFROM ${CUDA_IMAGE}\nRUN pip install -r requirements.txt\n"}) + f = cuda_named.findings(["cuda-torch-on-cpu"]) + assert_finding(f, "cannot-parse", "cannot be told") + assert_clean(f, "cuda-torch-on-cpu") # not silently cleared, not falsely flagged + # an unset ARG whose name carries no gpu word is equally unknown -- cannot-parse, not a CPU finding + plain = Fixture({"requirements.txt": REQ_TORCH, "Dockerfile": "ARG BASE\nFROM ${BASE}\nRUN pip install -r requirements.txt\n"}) + pf = plain.findings(["cuda-torch-on-cpu"]) + assert_finding(pf, "cannot-parse", "cannot be told") + assert_clean(pf, "cuda-torch-on-cpu") + + +@case("cuda-torch: an unresolved TAG over a real GPU name (`nvidia/cuda:${TAG}`) is still GPU, not cannot-parse") +def _(): + fx = Fixture({"requirements.txt": REQ_TORCH, "Dockerfile": "ARG TAG\nFROM nvidia/cuda:${TAG}\nRUN pip install -r requirements.txt\n"}) + assert_clean(fx.findings(["cuda-torch-on-cpu"])) # the resolved name settles GPU-ness; the tag is irrelevant to it + + +@case("cuda-torch: a placeholder left inside an unresolved NESTED default (`${IMAGE:-${GPU_BASE}}`) is cannot-parse, not a CPU finding") +def _(): + # _expand_args does not recurse into the nested default, so `${GPU_BASE}` + # survives expansion; stripping it must not make the stage read as a known + # CPU image (Bugbot, .github#459) -- the image is genuinely unknown. + fx = Fixture({"requirements.txt": REQ_TORCH, "Dockerfile": "ARG IMAGE\nFROM ${IMAGE:-${GPU_BASE}}\nRUN pip install -r requirements.txt\n"}) + f = fx.findings(["cuda-torch-on-cpu"]) + assert_finding(f, "cannot-parse", "cannot be told") + assert_clean(f, "cuda-torch-on-cpu") + + +@case("cuda-torch: a placeholder in only the REGISTRY (`${REGISTRY}/python:3.11-slim`) keeps the known image name -- a finding, not cannot-parse") +def _(): + # The image name (`python`) survives the strip, so GPU-or-CPU is known: this + # is a CPU install with the real "+cpu / index" remedy, not an unknown image + # (Bugbot, .github#459 nit -- unknown means no resolved NAME, not any placeholder). + fx = Fixture({"requirements.txt": REQ_TORCH, "Dockerfile": "ARG REGISTRY\nFROM ${REGISTRY}/python:3.11-slim\nRUN pip install -r requirements.txt\n"}) + f = fx.findings(["cuda-torch-on-cpu"]) + assert_finding(f, "cuda-torch-on-cpu", count=1) + assert_clean(f, "cannot-parse") + + @case("node: autoprefixer is reached by a postcss config, never by its own package.json key") def _(): alone = Fixture({"package.json": PKG % ('"react": "^18"', '"autoprefixer": "^10"'), "src/a.jsx": "import React from 'react';\n"})