Skip to content

fix: Small changes affecting packaging and cleanup #2641

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Syedowais312 wants to merge 3 commits into from
Closed

fix: Small changes affecting packaging and cleanup #2641

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4e7c7ee
fix(packaging): Cleanup artifacts on remove and purge
Syedowais312 Apr 2, 2026
9228588
build(goreleaser): Add 386 architecture targets
Syedowais312 Feb 17, 2026
a1c09e4
build(goreleaser): Install cleanup helper and enable plugins
Syedowais312 Feb 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .goreleaser-stable.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,13 @@ nfpms:
dst: /usr/bin/kraftld
- src: docs/man/
dst: /usr/share/man/man1/
- src: scripts/cleanup-artifacts.sh
dst: /usr/libexec/kraftkit/cleanup-artifacts
file_info:
mode: 0755
scripts:
preremove: packaging/deb/prerm
postremove: packaging/deb/postrm

aurs:
- homepage: https://kraftkit.sh
Expand Down Expand Up @@ -246,6 +253,11 @@ builds:
#@ if bin == "kraft" and os == "linux" and arch == "amd64":
- xen
#@ end
ignore:
- goos: linux
goarch: 386
- goos: windows
goarch: 386
#@ end
#@ end
#@ end
Expand Down
12 changes: 12 additions & 0 deletions .goreleaser-staging.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,13 @@ nfpms:
dst: /usr/bin/kraftld
- src: docs/man/
dst: /usr/share/man/man1/
- src: scripts/cleanup-artifacts.sh
dst: /usr/libexec/kraftkit/cleanup-artifacts
file_info:
mode: 0755
scripts:
preremove: packaging/deb/prerm
postremove: packaging/deb/postrm

brews:
- name: kraftkit-staging
Expand Down Expand Up @@ -184,6 +191,11 @@ builds:
#@ if bin == "kraft" and os == "linux" and arch == "amd64":
- xen
#@ end
ignore:
- goos: linux
goarch: 386
- goos: windows
goarch: 386
Comment thread
Syedowais312 marked this conversation as resolved.
#@ end
#@ end
#@ end
Expand Down
24 changes: 24 additions & 0 deletions packaging/deb/postrm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#!/usr/bin/env bash
set -e


# postrm — executed by dpkg AFTER package removal
#
# Debian policy:
# remove --> keep user data
# purge --> remove user data
#
# Therefore we only delete configuration on "purge".
# This allows reinstalling kraftkit without losing settings.
#
# Script MUST NOT fail, dpkg would leave the package in a broken state.
# Always ignore deletion errors.

case "$1" in
purge)
# User-scoped config directory
rm -rf "$HOME/.config/kraftkit" || true
;;
esac

exit 0
22 changes: 22 additions & 0 deletions packaging/deb/prerm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#!/usr/bin/env bash
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using '#!/usr/bin/env bash' in maintainer scripts adds an unnecessary dependency on bash (and on /usr/bin/env) for uninstall paths. Debian policy commonly recommends /bin/sh for maintainer scripts unless bash-specific features are required; this script is POSIX-compatible and could use /bin/sh.

Suggested change
#!/usr/bin/env bash
#!/bin/sh

Copilot uses AI. Check for mistakes.
set -e


# prerm executed BEFORE package removal
#
# Some kraftkit runtimes keep sockets/mounts/artifacts alive.
# If not cleaned, dpkg removal may fail or leave broken state.
#
# We attempt best-effort cleanup but NEVER block uninstall.

CLEANUP="/usr/libexec/kraftkit/cleanup-artifacts"

case "$1" in
remove|purge)
if [ -x "$CLEANUP" ]; then
"$CLEANUP" || true
fi
;;
esac

exit 0
34 changes: 34 additions & 0 deletions scripts/cleanup-artifacts.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#!/bin/sh
set -e

# cleanup-artifacts
#
# Removes runtime/cache data generated by kraftkit.
#
# Design goals:
# - Idempotent (safe to run multiple times)
# - Works without optional dependencies (yq)
# - Never deletes unintended paths
# - Never fails package uninstall

CONFIG="${HOME}/.config/kraftkit/config.yaml"

# Default runtime directories created by KraftKit.
CACHE_DIR="${HOME}/.cache/kraftkit"
DATA_DIR="${HOME}/.local/share/kraftkit"
Comment on lines +14 to +18
Copy link
Copy Markdown
Member

@craciunoiuc craciunoiuc Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean up for other users also

Copy link
Copy Markdown
Author

@Syedowais312 Syedowais312 Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want cleanup to apply only to the invoking user, or also root’s home when KraftKit was used via sudo? I want to avoid removing unrelated users’ data



# If a config exists and yq is available, read custom paths.
# Fallback to defaults to ensure cleanup always succeeds.
if [ -f "$CONFIG" ] && command -v yq >/dev/null 2>&1; then
CACHE_DIR="$(yq '.paths.cache // "'"$CACHE_DIR"'"' "$CONFIG")"
DATA_DIR="$(yq '.paths.data // "'"$DATA_DIR"'"' "$CONFIG")"
Comment thread
Syedowais312 marked this conversation as resolved.
Comment on lines +24 to +25
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The config keys read here ('.paths.cache' and '.paths.data') don’t exist in KraftKit’s config schema (config/config.go defines paths.manifests, paths.sources and runtime_dir). As a result custom locations won’t be respected. Update the expressions to match the actual keys you need to clean (and note that the config directory itself can be controlled via env and may not be under $HOME/.config).

Suggested change
CACHE_DIR="$(yq '.paths.cache // "'"$CACHE_DIR"'"' "$CONFIG")"
DATA_DIR="$(yq '.paths.data // "'"$DATA_DIR"'"' "$CONFIG")"
CACHE_DIR="$(yq '.runtime_dir // "'"$CACHE_DIR"'"' "$CONFIG")"
DATA_DIR="$(yq '.paths.sources // "'"$DATA_DIR"'"' "$CONFIG")"

Copilot uses AI. Check for mistakes.
Comment on lines +24 to +25
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this script uses 'set -e', a failing 'yq' invocation (unsupported yq flavor/version, parse error, etc.) will abort the script, contradicting the “never fails package uninstall” goal. Make yq reads best-effort (capture failures and fall back to defaults), and consider using yq raw output mode to avoid quoted/scalar formatting issues.

Suggested change
CACHE_DIR="$(yq '.paths.cache // "'"$CACHE_DIR"'"' "$CONFIG")"
DATA_DIR="$(yq '.paths.data // "'"$DATA_DIR"'"' "$CONFIG")"
NEW_CACHE_DIR="$(yq -r '.paths.cache // ""' "$CONFIG" 2>/dev/null || printf '')"
[ -n "$NEW_CACHE_DIR" ] && CACHE_DIR="$NEW_CACHE_DIR"
NEW_DATA_DIR="$(yq -r '.paths.data // ""' "$CONFIG" 2>/dev/null || printf '')"
[ -n "$NEW_DATA_DIR" ] && DATA_DIR="$NEW_DATA_DIR"

Copilot uses AI. Check for mistakes.
fi


# Safety guards:
# - non empty
# - not root directory
# - ignore missing paths
[ -n "$CACHE_DIR" ] && [ "$CACHE_DIR" != "/" ] && rm -rf -- "$CACHE_DIR" || true
[ -n "$DATA_DIR" ] && [ "$DATA_DIR" != "/" ] && rm -rf -- "$DATA_DIR" || true
Comment on lines +14 to +34
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This helper relies on $HOME to locate/remove artifacts. When invoked from package maintainer scripts it typically runs as root (HOME=/root), so it will miss artifacts for real users and/or delete root’s cache instead. Consider iterating over actual user home dirs (e.g., from /etc/passwd) and removing per-user paths relative to each home, or accept an explicit target home/user and have the maintainer script call it appropriately.

Suggested change
CONFIG="${HOME}/.config/kraftkit/config.yaml"
# Default runtime directories created by KraftKit.
CACHE_DIR="${HOME}/.cache/kraftkit"
DATA_DIR="${HOME}/.local/share/kraftkit"
# If a config exists and yq is available, read custom paths.
# Fallback to defaults to ensure cleanup always succeeds.
if [ -f "$CONFIG" ] && command -v yq >/dev/null 2>&1; then
CACHE_DIR="$(yq '.paths.cache // "'"$CACHE_DIR"'"' "$CONFIG")"
DATA_DIR="$(yq '.paths.data // "'"$DATA_DIR"'"' "$CONFIG")"
fi
# Safety guards:
# - non empty
# - not root directory
# - ignore missing paths
[ -n "$CACHE_DIR" ] && [ "$CACHE_DIR" != "/" ] && rm -rf -- "$CACHE_DIR" || true
[ -n "$DATA_DIR" ] && [ "$DATA_DIR" != "/" ] && rm -rf -- "$DATA_DIR" || true
cleanup_user_home() {
user_home=$1
# Skip if home is empty or not a directory.
[ -n "$user_home" ] && [ -d "$user_home" ] || return 0
CONFIG="${user_home}/.config/kraftkit/config.yaml"
# Default runtime directories created by KraftKit for this user.
CACHE_DIR="${user_home}/.cache/kraftkit"
DATA_DIR="${user_home}/.local/share/kraftkit"
# If a config exists and yq is available, read custom paths.
# Fallback to defaults to ensure cleanup always succeeds.
if [ -f "$CONFIG" ] && command -v yq >/dev/null 2>&1; then
CACHE_DIR="$(yq '.paths.cache // "'"$CACHE_DIR"'"' "$CONFIG")"
DATA_DIR="$(yq '.paths.data // "'"$DATA_DIR"'"' "$CONFIG")"
fi
# Safety guards:
# - non empty
# - not root directory
# - ignore missing paths
[ -n "$CACHE_DIR" ] && [ "$CACHE_DIR" != "/" ] && rm -rf -- "$CACHE_DIR" || true
[ -n "$DATA_DIR" ] && [ "$DATA_DIR" != "/" ] && rm -rf -- "$DATA_DIR" || true
}
# When run as root (e.g., from maintainer scripts), iterate over real users'
# home directories. Otherwise, clean only the current user's home.
uid=$(id -u 2>/dev/null || echo 1)
if [ "$uid" -eq 0 ]; then
# Root: iterate over users from /etc/passwd (UID >= 1000, exclude nobody 65534).
if [ -r /etc/passwd ]; then
while IFS=: read -r _ _ pw_uid _ _ pw_home _; do
# Ensure UID is numeric.
case "$pw_uid" in
''|*[!0-9]*) continue ;;
esac
if [ "$pw_uid" -ge 1000 ] && [ "$pw_uid" -ne 65534 ]; then
[ -n "$pw_home" ] && cleanup_user_home "$pw_home"
fi
done < /etc/passwd
fi
else
# Non-root: operate on the current user's home.
cleanup_user_home "${HOME:-}"
fi

Copilot uses AI. Check for mistakes.
Comment on lines +29 to +34
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current safety guard only prevents empty and '/' paths. If the config points at '.'/'..', '/tmp', or another broad directory, this will recursively delete unintended data. Add stricter validation (e.g., require absolute paths under the expected base dir, reject '.', '..', and paths not ending in '/kraftkit', and/or resolve/verify with realpath before rm -rf).

Suggested change
# Safety guards:
# - non empty
# - not root directory
# - ignore missing paths
[ -n "$CACHE_DIR" ] && [ "$CACHE_DIR" != "/" ] && rm -rf -- "$CACHE_DIR" || true
[ -n "$DATA_DIR" ] && [ "$DATA_DIR" != "/" ] && rm -rf -- "$DATA_DIR" || true
# Safely remove a directory if it looks like an intended KraftKit path.
# Safety guards:
# - non empty
# - not root directory, '.', or '..'
# - absolute path
# - resolved (if realpath available)
# - under $HOME
# - path ends with '/kraftkit'
safe_rm() {
dir="$1"
# Ignore empty.
[ -z "$dir" ] && return 0
# Reject obviously dangerous values.
case "$dir" in
/|.|..|"") return 0 ;;
esac
# Require absolute paths.
case "$dir" in
/*) ;;
*) return 0 ;;
esac
# Resolve path if realpath is available.
if command -v realpath >/dev/null 2>&1; then
resolved="$(realpath -- "$dir" 2>/dev/null || true)"
[ -z "$resolved" ] && return 0
else
resolved="$dir"
fi
# Final sanity checks on the resolved path.
case "$resolved" in
/|.|..|"") return 0 ;;
esac
# Require the path to be under the user's home directory.
case "$resolved" in
"$HOME"/*) ;;
*) return 0 ;;
esac
# Require the directory name to be 'kraftkit' (to avoid wiping broader dirs).
case "$resolved" in
*/kraftkit) ;;
*) return 0 ;;
esac
rm -rf -- "$resolved"
}
# Ignore missing paths; safe_rm performs all other checks.
safe_rm "$CACHE_DIR" || true
safe_rm "$DATA_DIR" || true

Copilot uses AI. Check for mistakes.