Skip to content

security: [LOW] Add defensive validation to tmpdir cleanup in install.sh #2998

@louisgv

Description

@louisgv

Finding

File: sh/cli/install.sh (line 197-198)
Severity: LOW
Type: Defense-in-depth improvement

Details

The build_and_install() function uses mktemp and trap for cleanup:

tmpdir=$(mktemp -d)
trap 'rm -rf "${tmpdir}"' EXIT

Current behavior:

  • set -eo pipefail ensures script exits if mktemp fails (non-zero exit)
  • This prevents trap from executing with empty $tmpdir
  • The code is functionally safe

Defense-in-depth concern:

  • If mktemp somehow succeeds but returns empty string (extremely unlikely), trap would execute rm -rf ""
  • While quoted variable is safe in bash (expands to empty arg), best practice is explicit validation

Recommendation

Add defensive check before trap fires:

tmpdir=$(mktemp -d)
[ -n "$tmpdir" ] || { log_error "mktemp failed"; exit 1; }
trap '[ -n "$tmpdir" ] && [ -d "$tmpdir" ] && rm -rf "$tmpdir"' EXIT

Or inline in trap:

tmpdir=$(mktemp -d)
trap '[ -n "${tmpdir}" ] && [ -d "${tmpdir}" ] && rm -rf "${tmpdir}"' EXIT

Impact

  • Likelihood: Extremely low (mktemp would need to return 0 exit but empty string)
  • Current impact: None (quoted variable is safe)
  • Benefit: Defense-in-depth best practice for rm -rf operations

Notes

This is a low-priority hardening recommendation, not an active vulnerability. The current code is safe due to set -e.


Filed by shell-scanner (automated security scan)

Metadata

Metadata

Assignees

No one assigned

    Labels

    in-progressIssue is being actively worked onsafe-to-workSecurity triage: safe for automated processing

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions