Security hardening: Use mktemp for temporary files in diff tool to pr… #1957

krishnagamerz · 2025-12-26T12:46:08Z

…event symlink attacks

google-cla · 2025-12-26T12:46:12Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

gemini-code-assist · 2025-12-26T12:46:27Z

Summary of Changes

Hello @krishnagamerz, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the security and stability of the diff.bash script by addressing vulnerabilities related to temporary file and directory creation. It introduces the use of mktemp to generate unique and secure temporary locations for registry operations and configuration, thereby mitigating symlink attack vectors. Additionally, the changes improve the script's overall robustness through more consistent and secure variable handling practices.

Highlights

Security Hardening: Implemented mktemp -d for creating temporary directories, specifically for registry storage and configuration, to prevent potential symlink attacks.
Secure File Handling: Ensured temporary configuration files and certificates are generated within securely created, unique temporary directories with restrictive permissions (umask 077).
Robustness Improvements: Enhanced shell script reliability by consistently quoting variables (e.g., $STDERR, $BUILD_WORKSPACE_DIRECTORY, $image_label, $REPORT_FILE) to prevent word splitting and globbing issues.
Temporary Directory Cleanup: Added logic to the on_exit trap to automatically remove the REGISTRY_TMPDIR if it was created, ensuring proper cleanup of temporary resources.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request hardens the diff.bash script against symlink attacks by using mktemp to create temporary directories and files, instead of using predictable paths in /tmp. This is a great security improvement. The changes also consistently quote variable expansions, which improves script robustness.

I've found a few issues:

A regression where STDERR and CHANGED_IMAGES_FILE are initialized after the on_exit trap is set, which can cause the script to fail.
The temporary files for STDERR and CHANGED_IMAGES_FILE are not cleaned up on exit.
There is some code duplication when setting up the temporary registry that could be refactored.

I've left specific comments with suggestions for each of these points.

gemini-code-assist · 2025-12-26T12:47:44Z

private/tools/diff.bash

+    fi
    pkill -P $$
 }
 trap "on_exit" EXIT


The trap is set before STDERR and CHANGED_IMAGES_FILE are initialized with mktemp (lines 36-37). If the script exits for any reason between this line and line 37, the on_exit function will fail because it tries to access STDERR, which would be an unbound variable. This is especially problematic with set -o nounset.

To fix this, you should initialize STDERR and CHANGED_IMAGES_FILE before setting the trap.

gemini-code-assist · 2025-12-26T12:47:44Z

private/tools/diff.bash

+    if [[ -n "${REGISTRY_TMPDIR:-}" && -d "${REGISTRY_TMPDIR}" ]]; then
+        rm -rf "${REGISTRY_TMPDIR}"
+    fi
    pkill -P $$


The temporary files created for STDERR and CHANGED_IMAGES_FILE are not being cleaned up in the on_exit function. This can leave temporary files on the filesystem. Please add cleanup for these files.

rm -f "${STDERR:-}" "${CHANGED_IMAGES_FILE:-}" pkill -P $$

gemini-code-assist · 2025-12-26T12:47:44Z

private/tools/diff.bash

 if [[ "${REGISTRY}" == "spawn_https" ]]; then
    # Make a self signed cert
-    rm -f /tmp/localhost.pem /tmp/localhost-key.pem
-    rm -rf $DISK_STORAGE
+    umask 077
+    REGISTRY_TMPDIR="$(mktemp -d)"
+    DISK_STORAGE="${REGISTRY_TMPDIR}/diff-storage"
+    CFG_JSON="${REGISTRY_TMPDIR}/cfg.json"
+    CERT_PATH="${REGISTRY_TMPDIR}/localhost.pem"
+    KEY_PATH="${REGISTRY_TMPDIR}/localhost-key.pem"
+    rm -rf "${DISK_STORAGE}"
+    mkdir -p "${DISK_STORAGE}"
    mkcert -install
-    (cd /tmp && mkcert localhost)
-    echo '{
-        "http":{
-            "address":"127.0.0.1", "port":"4564",
-            "tls": {
-                "cert":"/tmp/localhost.pem",
-                "key":"/tmp/localhost-key.pem"
-            }
-        },
-        "log": { "level": "info" },
-        "storage":{"rootDirectory":"/tmp/diff-storage"}
-    }' >/tmp/cfg.json
+    mkcert -cert-file "${CERT_PATH}" -key-file "${KEY_PATH}" localhost
+    cat >"${CFG_JSON}" <<EOF
+{
+  "http": {
+    "address": "127.0.0.1",
+    "port": "4564",
+    "tls": {
+      "cert": "${CERT_PATH}",
+      "key": "${KEY_PATH}"
+    }
+  },
+  "log": { "level": "info" },
+  "storage": { "rootDirectory": "${DISK_STORAGE}" }
+}
+EOF
    REGISTRY="localhost:4564"
-    zot serve /tmp/cfg.json 1>&2 &
+    zot serve "${CFG_JSON}" 1>&2 &
    sleep 1
 fi

 if [[ "${REGISTRY}" == "spawn" ]]; then
-    rm -rf $DISK_STORAGE
-    mkdir $DISK_STORAGE
+    umask 077
+    REGISTRY_TMPDIR="$(mktemp -d)"
+    DISK_STORAGE="${REGISTRY_TMPDIR}/diff-storage"
+    mkdir -p "${DISK_STORAGE}"
    REGISTRY="localhost:4564"
    crane registry serve --address "$REGISTRY" --disk "$DISK_STORAGE" &
 fi


There's some code duplication between the spawn_https and spawn blocks. You can refactor this to reduce duplication and improve maintainability. The common logic for creating a temporary directory and storage path can be extracted.

if [[ "${REGISTRY}" == "spawn_https" || "${REGISTRY}" == "spawn" ]]; then umask 077 REGISTRY_TMPDIR="$(mktemp -d)" DISK_STORAGE="${REGISTRY_TMPDIR}/diff-storage" mkdir -p "${DISK_STORAGE}" REGISTRY="localhost:4564" fi if [[ "${REGISTRY}" == "spawn_https" ]]; then # Make a self signed cert CFG_JSON="${REGISTRY_TMPDIR}/cfg.json" CERT_PATH="${REGISTRY_TMPDIR}/localhost.pem" KEY_PATH="${REGISTRY_TMPDIR}/localhost-key.pem" mkcert -install mkcert -cert-file "${CERT_PATH}" -key-file "${KEY_PATH}" localhost cat >"${CFG_JSON}" <<EOF { "http": { "address": "127.0.0.1", "port": "4564", "tls": { "cert": "${CERT_PATH}", "key": "${KEY_PATH}" } }, "log": { "level": "info" }, "storage": { "rootDirectory": "${DISK_STORAGE}" } } EOF zot serve "${CFG_JSON}" 1>&2 & sleep 1 fi if [[ "${REGISTRY}" == "spawn" ]]; then crane registry serve --address "$REGISTRY" --disk "$DISK_STORAGE" & fi

…event symlink attacks

gemini-code-assist bot reviewed Dec 26, 2025

View reviewed changes

ca2vidd added 2 commits December 26, 2025 18:38

Security hardening: Use mktemp for temporary files in diff tool to pr…

b5ff218

…event symlink attacks

Fix trap ordering and cleanup temp files

cbff088

krishnagamerz force-pushed the security/mktemp-diff-registry branch from 0fe27ed to cbff088 Compare December 26, 2025 12:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Security hardening: Use mktemp for temporary files in diff tool to pr… #1957

Security hardening: Use mktemp for temporary files in diff tool to pr… #1957

Uh oh!

krishnagamerz commented Dec 26, 2025

Uh oh!

google-cla bot commented Dec 26, 2025

Uh oh!

gemini-code-assist bot commented Dec 26, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Dec 26, 2025

Uh oh!

gemini-code-assist bot Dec 26, 2025

Uh oh!

gemini-code-assist bot Dec 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Security hardening: Use mktemp for temporary files in diff tool to pr… #1957

Are you sure you want to change the base?

Security hardening: Use mktemp for temporary files in diff tool to pr… #1957

Uh oh!

Conversation

krishnagamerz commented Dec 26, 2025

Uh oh!

google-cla bot commented Dec 26, 2025

Uh oh!

gemini-code-assist bot commented Dec 26, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants