[devin-ai-integration]

Summary

Fixes the four SonarCloud docker:S6506 alerts by enforcing HTTPS on the VICAR release downloads. The four docker:S6471 (root default user) alerts are left unfixed on purpose: switching any of these images to a non-root default breaks documented workflows today, and the changes needed to make it safe are in files owned by other in-flight security PRs (see below).

-    curl -L "https://github.com/NASA-AMMOS/VICAR/releases/download/..." -o ...
+    curl --proto "=https" --tlsv1.2 -fL "https://github.com/NASA-AMMOS/VICAR/releases/download/..." -o ...

--proto =https restricts the first request and every redirect to HTTPS (GitHub redirects release assets to release-assets.githubusercontent.com, which stays HTTPS). --tlsv1.2 sets a TLS floor. -f makes a 4xx/5xx fail the RUN step instead of passing an HTML error page to tar. The image contents don't change.

Alert mapping

Alert	Rule	Location	Resolution
#8	S6506	docker/Dockerfile:26	Fixed: --proto "=https" --tlsv1.2 -fL
#9	S6506	docker/Dockerfile:53	Fixed: same
#12	S6506	builder/Dockerfile:53	Fixed: same
#13	S6506	builder/Dockerfile:65	Fixed: same
#65	S6471	docker/Dockerfile:99 (runtime)	Not fixed. See below. Recommended: keep open until the follow-up lands, or dismiss as "won't fix"
#66	S6471	builder/Dockerfile:14	Not fixed. Same reasons. Same recommendation
#69	S6471	fullfeatured/Dockerfile:33	Not fixed. Same reasons. Same recommendation
#70	S6471	visor/Dockerfile:22	Not fixed. Same reasons. Same recommendation

Why the non-root default (S6471) isn't in this PR

These are interactive tool containers that write output into a bind-mounted host directory. The tig CLI already drops root where it can: on Linux it runs with --user $(id -u):$(id -g) (Docker) or --userns keep-id (rootless Podman), in spec.py build_run_spec and build.py. A USER line in the image doesn't change those paths. It does break every path that relies on the image default:

1. macOS + Podman, the documented macOS setup (docs/install-macos.md). spec.py and build.py skip --user/keep-id when sys.platform == "darwin", so the container runs as the image's default user. podman machine is rootless Podman in a Linux VM, and only container root maps to the VM user that owns the shared home directory. Reproduced with rootless Podman on Linux, using a workspace owned by the invoking user:

   podman run -v $D:/workspace oraclelinux:8 touch /workspace/f                    # OK (container root)
   podman run --user 1000:1000 -v $D:/workspace oraclelinux:8 touch /workspace/f   # Permission denied
   podman run --userns keep-id -v $D:/workspace oraclelinux:8 touch /workspace/f   # OK
   

   tig and tig --build (builder image) would no longer be able to write outputs on macOS/Podman.
2. Direct docker run -v $PWD:/workspace ..., as documented in the release notes generated by build-publish-terrain-intelligence-generator.yml, in terrain-intelligence-generator/README.md/docs/demos/building-from-source.md (builder), and in build-builder-image.sh. A fixed image uid only works when it happens to match the host uid. With a mktemp -d workspace owned by uid 1001 (the GitHub runner uid), docker run --user 1000:1000 -v $D:/workspace → Permission denied.
3. CI image tests: test-visor-image.sh (MSL marsrad step), test-fullfeatured-image.sh, test-docker-image.sh, test-product-pipeline.sh, and the inline tests in the build workflow all write into a mktemp -d bind mount without --user. The visor and fullfeatured variant workflows run their tests on pull requests, so this PR would go red.

All the files needed to make a non-root default safe (tig-cli/src/tig_cli/spec.py, the workflows, the test-*.sh scripts) belong to the other security PRs running in parallel, so changing them here would cause conflicts. Suggested follow-up once those merge:

• in spec.py/build.py, pass --userns keep-id for Podman on macOS as well (Docker Desktop already maps ownership);
• add --user "$(id -u):$(id -g)" to the test scripts, the workflow's inline tests, and the documented docker run examples;
• then add a tig user (fixed uid) and USER tig to docker/ and builder/, and in visor/+fullfeatured/ switch USER root → install calibration → USER tig. Note that examples/airflow-k8s-pipeline/k8s/worker/Dockerfile already sets USER root before its RUN, so it isn't affected.

Verification

• Built both changed images locally (Maven mirror via a scratch settings.xml, not committed): docker/Dockerfile → EXIT 0, builder/Dockerfile → EXIT 0. Both VICAR tarballs downloaded through the new flags.
• ./terrain-intelligence-generator/test-docker-image.sh tig-runtime:https-test → 15 passed, 0 failed.
• Builder smoke test: docker run --user $(id -u):$(id -g) -v $PWD:/build <builder> bash -c 'cp $V2TOP/p2/prog/gen/gen.* /build/ && vicar-build gen' → gen compiled and linked.
• In oraclelinux:8 (curl 7.61.1): both release URLs resolve with the new flags (1 redirect, HTTPS, HTTP 206 on a range request). curl --proto "=https" refuses an http:// URL and an HTTPS→HTTP redirect (Protocol "http" not supported or disabled).
• No regression test was added. The check is enforced by the RUN step itself, and the only CI-wired places for an image test are the workflows/test-*.sh scripts owned by the parallel PRs.
• Not user-facing: same image contents, tags, build args and entrypoint, so no doc changes.

Link to Devin session: https://nasa-jpl-demo.devinenterprise.com/sessions/3b988076be934f7bb6332ef577d5a516
Open in Devin Desktop: https://nasa-jpl-demo.devinenterprise.com/desktop/session/3b988076be934f7bb6332ef577d5a516?variant=devin
Requested by: @HuiJun

---

🟢 Devin Review found nothing to fix · View findings

[devin-ai-integration]

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

• Disable automatic comment, CI, and merge conflict monitoring

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

[github-actions]

Project Requirements: PASS

PR #51 is a small hardening change. In terrain-intelligence-generator/docker/Dockerfile and builder/Dockerfile, the four VICAR release downloads change from curl -L to curl --proto "=https" --tlsv1.2 -fL. The URLs, build args (VICAR_VERSION, BINARIES_FILE, EXTERNAL_FILE), image contents, tags and entrypoint are unchanged. The only visible difference is that a failed download (HTTP error, or a redirect to a non-HTTPS URL) now stops at the curl step instead of failing later in tar. Documentation parity: pass. Broad deployability: pass. Test coverage: concern, because the builder image is not built by any CI workflow and no failure case is tested. The gate is not blocked.

Requirement	Verdict	Notes
1. Documentation parity	Pass	Effectively not user-facing. The image contents, tags, build args, bundled VICAR version and entrypoint are unchanged, and the asset URLs stay the same public GitHub release HTTPS URLs. The only visible change is that a failed download now stops at the curl step instead of producing a corrupt tarball that tar rejects. The existing troubleshooting entry in terrain-intelligence-generator/README.md ('A build download fails': check the version exists and set VICAR_VERSION/BINARIES_FILE/EXTERNAL_FILE) still describes that behavior correctly. No docs needed updating.
2. Broad deployability	Pass	No site-specific hosts, credentials, runtimes or platforms are introduced. The downloads still come from the public NASA-AMMOS/VICAR GitHub releases. --proto =https, --tlsv1.2 and -f are standard curl options, and the curl shipped in the oraclelinux:8 base image used by both Dockerfiles supports them. Requiring TLS 1.2 or newer and HTTPS-only redirects matches what GitHub already serves, so no legitimate user is excluded. A user's network setup that hands out a non-HTTPS redirect for these assets would now fail, which is the intended hardening.
3. Test coverage	Concern	The success path for docker/Dockerfile is exercised in CI. build-publish-terrain-intelligence-generator.yml triggers on 'terrain-intelligence-generator/docker/**' for pull requests and builds the image, so the new curl flags must successfully download both tarballs. builder/Dockerfile is not built by any workflow: it is local-only, and the integration tests in tig-cli/tests/integration/test_build_from_source.py skip when the builder image is absent. So that half of the change has no automated coverage. Nothing checks the new failure behavior (HTTP error or non-HTTPS redirect stops the build at curl). That is hard to test in a Dockerfile, so this is not blocking.

3. Test coverage

• terrain-intelligence-generator/builder/Dockerfile:53: The builder image is not built by any CI workflow, so a mistake in the changed download commands (curl flags rejected, URL typo) would only surface when a user runs build-builder-image.sh or tig --build. Fix: Add a PR-triggered CI job (path filter 'terrain-intelligence-generator/builder/**') that runs at least the download/unpack stage of builder/Dockerfile, for example with docker build --target or by building and then running vicar-build on one small unit. Alternatively, document a manual check in the README's builder section.
• terrain-intelligence-generator/docker/Dockerfile:26: No test checks the failure case this change introduces: a missing release asset (HTTP 404) should now stop the build at the curl step with a non-zero exit, instead of handing an HTML error page to tar. Fix: Optionally add a CI step or test script that builds with a nonexistent VICAR_VERSION or BINARIES_FILE and asserts the build fails at the download step (for example, by grepping the build log for 'curl: (22)'). This would also cover the README troubleshooting entry 'A build download fails'.

Automated pre-check hints

• terrain-intelligence-generator/ changed but no test scripts changed (requirement 3).

---

Evaluated at 6ed0a49 · Devin session · workflow run

Criteria: .github/project-requirements/criteria.md. A fail blocks the merge; a maintainer can waive it by adding the requirements-waived label.