Skip to content

Commit 854e431

Browse files
authored
Development: Improve input validation and CI supply chain hardening (#13549)
1 parent 77e6fb4 commit 854e431

12 files changed

Lines changed: 263 additions & 28 deletions

File tree

.github/workflows/ci-build.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ jobs:
318318
permissions:
319319
contents: read
320320
packages: write
321-
uses: ls1intum/.github/.github/workflows/build-and-push-docker-image.yml@v1.2.0
321+
uses: ls1intum/.github/.github/workflows/build-and-push-docker-image.yml@ee1b52f07891ed489887681c09c52b4a83fa9fb2 # v1.2.0
322322
with:
323323
# Check out the exact commit the sha-<commit> tag names (not the mutable branch tip), so image
324324
# content and the tag stay pinned to the same commit. The `type=ref` tags come from the event

.github/workflows/ci-workflows.yml

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,21 @@ jobs:
4343
steps:
4444
- uses: actions/checkout@v6
4545
- name: Run actionlint
46-
# Pin the install script to a release tag, not `main` — it executes a downloaded
47-
# binary. `-shellcheck=` disables integrated shellcheck, which flags pre-existing
48-
# style issues in the legacy deploy workflows.
46+
# The install script is pinned to a release tag AND to its SHA-256, because piping it
47+
# straight into bash would execute whatever that URL returns. A tag can be moved; the
48+
# checksum cannot, so verify before running. Bump both together when raising the version.
49+
# `-shellcheck=` disables integrated shellcheck, which flags pre-existing style issues
50+
# in the legacy deploy workflows.
4951
env:
5052
ACTIONLINT_VERSION: '1.7.12'
53+
ACTIONLINT_INSTALLER_SHA256: '72fa3e45ac20f3c3a512d6747b4fcf719e21f890e8c43e78d48a41fdfb900c4e'
5154
run: |
52-
bash <(curl --silent --location \
53-
"https://raw.githubusercontent.com/rhysd/actionlint/v${ACTIONLINT_VERSION}/scripts/download-actionlint.bash") \
54-
"${ACTIONLINT_VERSION}"
55+
curl --silent --show-error --fail --location \
56+
"https://raw.githubusercontent.com/rhysd/actionlint/v${ACTIONLINT_VERSION}/scripts/download-actionlint.bash" \
57+
--output download-actionlint.bash
58+
echo "${ACTIONLINT_INSTALLER_SHA256} download-actionlint.bash" | sha256sum --check --strict
59+
bash download-actionlint.bash "${ACTIONLINT_VERSION}"
60+
rm download-actionlint.bash
5561
./actionlint -color -shellcheck=
5662
5763
- name: Check version pins are in sync

.github/workflows/scorecard.yml

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,16 @@
33
# vulnerability handling, and so on).
44
#
55
# It reports on practices Artemis already follows, which is the point: it turns them into an
6-
# externally computed, checkable number instead of a claim in a document. Findings land in the
7-
# code-scanning UI next to CodeQL, and `publish_results: true` feeds the badge in README.md.
6+
# externally computed, checkable number instead of a claim in a document. `publish_results: true`
7+
# feeds the badge in README.md and makes the score independently verifiable via
8+
# api.securityscorecards.dev; the full findings are attached to each run as `results.sarif`.
9+
#
10+
# The results are deliberately NOT uploaded to code scanning. Scorecard measures repository
11+
# posture, not exploitable defects: it reports passing scores ("28/30 changesets approved") and
12+
# not-applicable checks ("project is not fuzzed") as `error`-severity alerts, ~155 of them. Next
13+
# to CodeQL that buries the findings that describe an actual vulnerability, which is the one thing
14+
# the code-scanning tab has to stay useful for. Its fingerprints also embed the changing score
15+
# text, so dismissed findings reappear as new alerts on the next run.
816
#
917
# This is advisory. It never gates a pull request, and it does not run on pull requests at all:
1018
# the analysis needs repository-level metadata that a fork PR token cannot read.
@@ -33,8 +41,7 @@ jobs:
3341
runs-on: ubuntu-latest
3442
timeout-minutes: 20
3543
permissions:
36-
security-events: write # upload the SARIF result to code scanning
37-
id-token: write # Sigstore OIDC, required to publish the result
44+
id-token: write # Sigstore OIDC, required to publish the result
3845
contents: read
3946
steps:
4047
- name: Checkout
@@ -51,14 +58,10 @@ jobs:
5158
# the README badge and lets anyone re-check the score without access to this repository.
5259
publish_results: true
5360

61+
# The full findings live here rather than in the code-scanning tab (see the header comment).
5462
- name: Upload artifact
5563
uses: actions/upload-artifact@v7
5664
with:
5765
name: scorecard-results
5866
path: results.sarif
59-
retention-days: 5
60-
61-
- name: Upload to code scanning
62-
uses: github/codeql-action/upload-sarif@v4
63-
with:
64-
sarif_file: results.sarif
67+
retention-days: 30

.github/workflows/test-android.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ concurrency:
3636
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
3737
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
3838

39+
permissions: {} # default-deny; the job grants only what it needs
40+
3941
jobs:
4042
e2e-tests:
4143
name: Android E2E Tests
@@ -50,6 +52,9 @@ jobs:
5052
github.event.pull_request.head.repo.full_name == github.repository
5153
runs-on: [self-hosted, ase-large-android-sdk-34]
5254
timeout-minutes: 60
55+
permissions:
56+
contents: read # check out this repo and the public artemis-android repo
57+
checks: write # dorny/test-reporter publishes the Android E2E results as a check run
5358

5459
steps:
5560
- name: Checkout Main Artemis Repo (this repo)

.github/workflows/testserver-deployment.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ jobs:
132132
permissions:
133133
contents: read
134134
packages: write
135-
uses: ls1intum/.github/.github/workflows/build-and-push-docker-image.yml@v1.2.0
135+
uses: ls1intum/.github/.github/workflows/build-and-push-docker-image.yml@ee1b52f07891ed489887681c09c52b4a83fa9fb2 # v1.2.0
136136
with:
137137
ref: ${{ needs.determine-build-context.outputs.head_sha }}
138138
image-name: ls1intum/artemis

.github/workflows/validate-pr-title.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,14 @@ concurrency:
88
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
99
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
1010

11+
permissions: {} # default-deny; the job grants only what it needs
12+
1113
jobs:
1214
validate-pr-title:
1315
runs-on: ubuntu-latest
1416
timeout-minutes: 1
17+
permissions:
18+
contents: read
1519
steps:
1620
- uses: Slashgear/action-check-pr-title@161cede0311ec624ae3ee76a2c27522f7b6fa2d8 # v5.0.1
1721
with:

docker/artemis/Dockerfile

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ ARG WAR_FILE_STAGE="builder"
2222
#-----------------------------------------------------------------------------------------------------------------------
2323
# build stage
2424
#-----------------------------------------------------------------------------------------------------------------------
25-
FROM --platform=$BUILDPLATFORM docker.io/library/eclipse-temurin:25.0.3_9-jdk AS builder
25+
FROM --platform=$BUILDPLATFORM docker.io/library/eclipse-temurin:25.0.3_9-jdk@sha256:32861ec22e54af9597a3875c69001f57c0954648f5e3fcb6be601b4e35290ab5 AS builder
2626

2727
# python3/build-essential were previously installed defensively for npm deps that
2828
# trigger node-gyp; no current Artemis dep needs that. If a future dep does, the
@@ -77,7 +77,7 @@ RUN \
7777
#-----------------------------------------------------------------------------------------------------------------------
7878
# external build stage
7979
#-----------------------------------------------------------------------------------------------------------------------
80-
FROM docker.io/library/alpine:3.23.5 AS external_builder
80+
FROM docker.io/library/alpine:3.23.5@sha256:fd791d74b68913cbb027c6546007b3f0d3bc45125f797758156952bc2d6daf40 AS external_builder
8181

8282
#default path of the built .war files
8383
ARG WAR_FILE_PATH="/opt/artemis/build/libs"
@@ -97,7 +97,7 @@ FROM ${WAR_FILE_STAGE} AS war_file
9797
#-----------------------------------------------------------------------------------------------------------------------
9898
# extract stage (explode the WAR so the runtime image runs from a flat classpath -> faster startup)
9999
#-----------------------------------------------------------------------------------------------------------------------
100-
FROM --platform=$BUILDPLATFORM docker.io/library/eclipse-temurin:25.0.3_9-jdk AS extractor
100+
FROM --platform=$BUILDPLATFORM docker.io/library/eclipse-temurin:25.0.3_9-jdk@sha256:32861ec22e54af9597a3875c69001f57c0954648f5e3fcb6be601b4e35290ab5 AS extractor
101101

102102
#default path of the built .war files
103103
ARG WAR_FILE_PATH="/opt/artemis/build/libs"
@@ -113,7 +113,7 @@ RUN java -Djarmode=tools -jar Artemis.war extract --destination /extract/app
113113
#-----------------------------------------------------------------------------------------------------------------------
114114
# runtime stage
115115
#-----------------------------------------------------------------------------------------------------------------------
116-
FROM docker.io/library/eclipse-temurin:25.0.3_9-jdk AS runtime
116+
FROM docker.io/library/eclipse-temurin:25.0.3_9-jdk@sha256:32861ec22e54af9597a3875c69001f57c0954648f5e3fcb6be601b4e35290ab5 AS runtime
117117

118118
#default path of the built .war files
119119
ARG WAR_FILE_PATH="/opt/artemis/build/libs"

docker/jenkins/Dockerfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
FROM docker.io/jenkins/jenkins:lts
1+
FROM docker.io/jenkins/jenkins:lts@sha256:8547df3b0db2803d158ecc9499207a056bb30c23fddc18bb5b4a4dc14e77dd09
22

33
LABEL description="Jenkins with plugins pre-installed for Artemis"
44

docker/weaviate/Dockerfile.embeddinggemma

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
FROM semitechnologies/transformers-inference:custom
1+
FROM semitechnologies/transformers-inference:custom@sha256:a4b990d92e94854e7bf6cf4b56ec67fcdfac3b74a14f5f638644e6f7c2ba4cbc
22

33
# embeddinggemma-300m is a gated model requiring Hugging Face authentication
44
# Pass HF_TOKEN as a BuildKit secret: docker buildx build --secret id=hf_token,src=path/to/token ...

src/main/java/de/tum/cit/aet/artemis/core/util/FileUtil.java

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import java.nio.charset.Charset;
1414
import java.nio.file.Files;
1515
import java.nio.file.Path;
16+
import java.nio.file.StandardOpenOption;
1617
import java.time.ZonedDateTime;
1718
import java.util.ArrayList;
1819
import java.util.Arrays;
@@ -280,6 +281,82 @@ public static void sanitizeByCheckingIfPathStartsWithSubPathElseThrow(@NonNull U
280281
}
281282
}
282283

284+
/**
285+
* Resolves a single filename against a base directory and guarantees that the result stays inside that directory.
286+
*
287+
* <p>
288+
* {@link #sanitizeFilename(String)} already replaces every path separator, so a sanitised name cannot traverse on
289+
* its own. This method adds the containment check at the point of use, which is what makes the guarantee local and
290+
* checkable: the resolved path is normalised and compared against the normalised base directory, so a caller that
291+
* forgets to sanitise — or a future change that loosens the sanitiser — fails loudly instead of quietly reading or
292+
* writing an arbitrary file.
293+
*
294+
* <p>
295+
* The containment this gives is <em>lexical</em>: it compares path elements and does not resolve symlinks, so a
296+
* link already present inside {@code baseDirectory} would still point elsewhere. Callers that create the file are
297+
* responsible for opening it with {@link java.nio.file.StandardOpenOption#CREATE_NEW}, which refuses to follow an
298+
* existing link, rather than relying on this check alone.
299+
*
300+
* @param baseDirectory the directory the resolved path has to stay within
301+
* @param filename the single filename to resolve against {@code baseDirectory}
302+
* @return the resolved, normalised path, guaranteed to lie inside {@code baseDirectory}
303+
* @throws IllegalArgumentException if the filename is blank or escapes {@code baseDirectory}
304+
*/
305+
@NonNull
306+
public static Path resolveWithinDirectoryElseThrow(@NonNull Path baseDirectory, @NonNull String filename) {
307+
if (filename.isBlank()) {
308+
throw new IllegalArgumentException("Invalid filename: must not be blank.");
309+
}
310+
Path normalisedBaseDirectory = baseDirectory.normalize();
311+
Path resolvedPath = normalisedBaseDirectory.resolve(filename).normalize();
312+
// startsWith() compares path elements, not characters, so a sibling directory sharing a name prefix cannot pass.
313+
if (!resolvedPath.startsWith(normalisedBaseDirectory) || resolvedPath.equals(normalisedBaseDirectory)) {
314+
throw new IllegalArgumentException("Invalid filename '%s': the resolved path escapes the expected directory.".formatted(filename));
315+
}
316+
return resolvedPath;
317+
}
318+
319+
/**
320+
* Writes a stream to a path that must not exist yet, creating any missing parent directories.
321+
*
322+
* <p>
323+
* Opens with {@link StandardOpenOption#CREATE_NEW}, which maps to {@code O_CREAT | O_EXCL}. The kernel refuses
324+
* that combination when the path already exists — including when it exists only as a symlink, and including a
325+
* dangling one — so the write cannot be redirected through a link planted at the destination. This is the
326+
* companion to {@link #resolveWithinDirectoryElseThrow(Path, String)}, whose containment check is lexical and
327+
* therefore blind to symlinks on its own.
328+
*
329+
* @param inputStream the stream to write; closed by the caller
330+
* @param target the file to create
331+
* @throws java.nio.file.FileAlreadyExistsException if {@code target} already exists, symlink included
332+
* @throws IOException if creating the directories or writing fails
333+
*/
334+
public static void writeNewFileElseThrow(@NonNull InputStream inputStream, @NonNull Path target) throws IOException {
335+
Path parent = target.getParent();
336+
if (parent != null) {
337+
Files.createDirectories(parent);
338+
}
339+
// Opened outside the try so that a failure of the open itself is NOT cleaned up: it throws
340+
// FileAlreadyExistsException precisely when the path is somebody else's file or symlink, and that is the case
341+
// this method exists to protect. Only once the open succeeds is the file ours to delete.
342+
OutputStream outputStream = Files.newOutputStream(target, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE);
343+
try (outputStream) {
344+
inputStream.transferTo(outputStream);
345+
}
346+
catch (IOException | RuntimeException e) {
347+
// CREATE_NEW creates the file before any byte is copied, so a failure part-way through would otherwise leave
348+
// a truncated file behind. The caller only registers the path for deletion after this method returns, so
349+
// nothing else would ever remove it.
350+
try {
351+
Files.deleteIfExists(target);
352+
}
353+
catch (IOException cleanupFailure) {
354+
e.addSuppressed(cleanupFailure);
355+
}
356+
throw e;
357+
}
358+
}
359+
283360
/**
284361
* Sanitizes a file path by checking for invalid characters or path traversal.
285362
*

0 commit comments

Comments
 (0)