Skip to content

fix: harden license-headers.sh symlink handling against set -e interaction #714

@aitestino

Description

@aitestino

Description

PR #655 added symlink skipping to scripts/license-headers.sh, but the implementation uses a fragile pattern to work around set -e:

validate_file "$file" || validate_result=$?
validate_result=${validate_result:-0}

This relies on a bash subtlety: || validate_result=$? prevents set -e from exiting on non-zero return, but only because the || operator makes the compound command succeed. If someone refactors this to a different structure (e.g., if ! validate_file "$file"; then ...), the set -e behavior changes.

Additionally, the magic return code 2 for symlinks is undocumented at the top of the script.

Proposed Solution

Use explicit conditional instead of || workaround:

# At top of script, document return codes
readonly VALIDATE_OK=0
readonly VALIDATE_ERROR=1
readonly VALIDATE_SYMLINK=2

# In the loop
if validate_file "$file"; then
    validate_result=0
else
    validate_result=$?
fi

if [[ $validate_result -eq $VALIDATE_SYMLINK ]]; then
    echo -e "${YELLOW}${NC} $file (symlink, skipped)"
    files_skipped=$((files_skipped + 1))
    continue
elif [[ $validate_result -ne $VALIDATE_OK ]]; then
    files_failed=$((files_failed + 1))
    failed_files+=("$file")
    continue
fi

Context

Introduced in #655. The current code works but is fragile. From the Bash manual, set -e has complex interactions with || operators that can surprise maintainers.

Metadata

Metadata

Assignees

No one assigned

    Labels

    code-qualityCode quality improvements and standards enforcementprio: low

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions