Skip to content

base-image preflight: the Dockerfile parser's divergences from Docker all fail open #392

Description

@swinney

Context

_instruction_text / _final_stage_base in src/cli/managers/base_image_preflight.py decide
which base image a service template's final stage runs on. That decision gates archi create:
a template whose final stage is not a probeable a2rchi base must be refused before
remove_existing_deployment() (src/cli/cli_main.py:294) destroys the operator's deployment.

The parser is a line-oriented approximation of Dockerfile syntax. Automated review of PR #387
found six distinct inputs where it diverges from Docker, across two rounds:

Input Divergence
RUN \ then <<TAG heredoc opened on a continuation, delimiter not recorded
TAG as terminator line.strip() closed <<TAG early
FROM ... ^FROM missed an indented instruction
RUN echo "example <<TAG here" quoted marker over-read as an opener
RUN << TAG, RUN 3<<TAG, RUN <<- TAG opener pattern only matched the tight form
# escape= + backtick continuation continuation character hard-coded to \

Every one failed the same direction: open. The template read as covered on an earlier
a2rchi builder stage while its real final stage was third-party, so the deploy proceeded
without probing the image that actually ships. All six are fixed and guarded on #387
(1e73e727, 8d7eb9c0).

The problem this issue tracks

The individual fixes are not the concern — they are landed and tested. The concern is that
each round of review found more, and none was the last one. The remaining surface includes
at least: ARG-substituted references (FROM $BASE), COPY --from provenance, quoting cases
the widened opener pattern still misreads, and opener forms nobody has enumerated.

#387 added a partial mitigation: an unterminated heredoc now fails closed
(_instruction_text returns None → template reported uncoverable → refusal). That covers
over-reads — the parser seeing an opener that is not one. It does not cover under-reads,
where the parser misses a real opener entirely and scans the payload as instructions. Both of
round 2's findings were under-reads.

What to decide

Pick one; this needs a human call, which is why it is not on #387.

  1. Differential-test the parser against BuildKit. Generate Dockerfiles, compare
    _final_stage_base against what docker build --print/BuildKit's parser reports as the
    final stage. Turns "we found six" into "we searched". Costs a Docker dependency in CI —
    note the gate is pytest tests/unit/ only, so this likely belongs in a separate job.
  2. Property-test the invariant instead of the syntax. Hypothesis over generated templates
    asserting the one property that matters: if the real final stage is not an a2rchi base,
    _final_stage_base must not report one.
    No Docker dependency, but the generator encodes
    the same assumptions the parser does.
  3. Delegate parsing. Use a real Dockerfile parser (e.g. dockerfile-parse) instead of
    regexes. Note CLAUDE.md says avoid third-party dependencies unless strictly necessary,
    and this one would sit on a deploy-critical path.
  4. Accept and bound it. Decide the current fail-closed rule plus the six guards is enough,
    and record that decision in the module docstring so the next reviewer does not re-derive it.

Acceptance

  • A recorded decision among the four, with the reasoning, in this issue.
  • If 1–3: implemented, with the six known inputs above kept as explicit regression tests
    (they are already in tests/unit/test_base_image_preflight.py; do not delete them).
  • If 4: the bound stated in _final_stage_base's docstring in the module's existing
    "state the bound rather than imply totality" idiom.

Pointers

Metadata

Metadata

Assignees

No one assigned

    Labels

    P3Priority: when possibleenhancementNew feature or requestneeds-humanBlocked: needs a human design decision

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions