Skip to content

Fix the 2026-10-17 review's medium findings (docs, Dockerfile lint in CI) - #4

Merged
jschuller merged 3 commits into
mainfrom
fix/review-mediums-2026-10-17
Oct 22, 2026
Merged

jschuller merged 3 commits into
mainfrom
fix/review-mediums-2026-10-17

Conversation

@jschuller

Copy link
Copy Markdown
Owner

The two medium findings from the 2026-10-17 repository review that this repo can fix without an owner decision. A re-check on 2026-10-20 found 4 of the 12 mediums already fixed by #2. The remaining ones need an owner decision, an instance, or changes to vendor files; they're listed below and not applied.

Finding Fix Commit
The docs contradicted each other on META-INF/ (CLAUDE.md:56 vs :107, CHANGELOG) The docs now say it's gitignored and untracked (it is, per .gitignore and git ls-files), and how to check the vendor files against the signed recipe zip. Whether it's tracked is unchanged ac7ce82
No docker-build smoke check in CI (partly fixed by #2) scripts/check_dockerfile.sh plus a CI job. It runs BuildKit's build checks (docker build --call=check) without building, so it needs no instance and no installer download. The checks frontend docker/dockerfile:1.27.1 (2026-09-30) is pinned by digest 1e0a7f8
(review follow-up) Accepted findings matched with no count Each known finding is accepted once, as a multiset (comm -23). A second --platform constant, or a repeated password ENV, now fails 0b154f1

A full build can't run in CI: the final stage copies the MID installer, which the first stage downloads from install.service-now.com and signature-checks.

Verified locally:

  • get_mid_url_test.sh 28/28 and check_dockerfile_test.sh 13/13 pass, both with a stubbed docker/curl.
  • shellcheck-py 0.10.0.1 is clean on every script.
  • The real check passes on Docker 29.8.2 / buildx 0.37.2 and reports the 3 known findings.
  • An adversarial probe failed with both new findings listed: a copy of the Dockerfile with an extra FROM --platform=linux/amd64 and ENV MID_INSTANCE_PASSWORD=abc.
  • The pinned digest matches docker buildx imagetools inspect docker/dockerfile:1.27.1.
  • No vendor file changed (asset/, Dockerfile vs Dockerfile.unmodified).
  • gitleaks is clean.

Not verified before this PR's CI: the new job on ubuntu-latest. It pulls the frontend and base-image metadata anonymously from Docker Hub, so a rate limit can fail it intermittently. Dependabot won't update the digest-pinned frontend.

Owner decisions (not applied)

  • Relying on MID auto-upgrade (CLAUDE.md:75, README.md:97-98), although the vendor advises disabling it in containers.
  • A secrets file instead of --env-file in the quickstart (README.md:122-126). MID_SECRETS_FILE is already supported at Dockerfile:94.
  • The quickstart MID user gets itom_admin and discovery_admin (README.md:115) rather than mid_server alone. It needs a MID host and a PDI to prove the minimum.
  • Whether to track META-INF/.
  • Pinning the signer certificate fingerprint (asset/validate_signature.sh:27-32, a vendor file): add a check outside asset/, and/or report it upstream.
  • The unsalted MD5 env hash, which covers the passwords (asset/calculate_mid_env_hash.sh:27-30, asset/init:349, vendor files): work around it outside asset/ (for example with MID_SECRETS_FILE), or report it upstream.

CRB2205 and others added 3 commits October 21, 2026 21:56
CLAUDE.md's gotchas said the vendor files are covered by META-INF/MANIFEST.MF and the
bump procedure replaced META-INF/*, while its repository structure and .gitignore say
META-INF/ is untracked; the changelog listed it among the refreshed recipe files. The
docs now agree with .gitignore and say how to verify the vendor files against the
signed recipe zip.

Co-Authored-By: Claude <noreply@anthropic.com>
A full image build cannot run in CI: the final stage copies the MID installer that the
first stage downloads from install.service-now.com. scripts/check_dockerfile.sh runs
docker build --call=check instead, which executes no RUN step, with the checks frontend
pinned by digest (docker/dockerfile 1.27.1). It accepts only the three known findings, the
deliberate linux/amd64 FROM pin and the vendor recipe's empty password ENV placeholders,
and fails on any other finding or on a Dockerfile it cannot check. A new CI job runs it;
tests/check_dockerfile_test.sh covers it with a stubbed docker, and shellcheck now covers
scripts/*.sh and tests/*.sh.

Co-Authored-By: Claude <noreply@anthropic.com>
@jschuller
jschuller merged commit 2e0a250 into main Oct 22, 2026
3 checks passed
@jschuller
jschuller deleted the fix/review-mediums-2026-10-17 branch October 22, 2026 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants