Skip to content

Security Fix: Correct directory permissions in logs_download.go (Alerts #461, #460)#8855

Merged
pelikhan merged 1 commit intomainfrom
main-33656aa3b954d106
Jan 4, 2026
Merged

Security Fix: Correct directory permissions in logs_download.go (Alerts #461, #460)#8855
pelikhan merged 1 commit intomainfrom
main-33656aa3b954d106

Conversation

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented Jan 4, 2026

Security Fix: Incorrect Directory Permissions

Alert Numbers: #461, #460
Severity: Medium
Rule: G301 - Expect directory permissions to be 0750 or less
File: pkg/cli/logs_download.go (lines 153, 160)

Vulnerability Description

The security scanner identified that directories were being created with overly permissive permissions (0755), which allows world-readable access. This violates the principle of least privilege and could potentially expose sensitive log data to unauthorized users on shared systems.

Affected locations:

  • Line 153: os.MkdirAll(destPath, 0755) - when creating artifact directories
  • Line 160: os.MkdirAll(filepath.Dir(destPath), 0755) - when creating parent directories for files

Fix Applied

Changed directory permissions from 0755 to 0750 in both locations within the flattenUnifiedArtifact function:

Before:

// Line 153
if err := os.MkdirAll(destPath, 0755); err != nil {
    return fmt.Errorf("failed to create directory %s: %w", destPath, err)
}

// Line 160
if err := os.MkdirAll(filepath.Dir(destPath), 0755); err != nil {
    return fmt.Errorf("failed to create parent directory for %s: %w", destPath, err)
}

After:

// Line 153
// Create directory in destination with owner+group permissions only (0750)
if err := os.MkdirAll(destPath, 0750); err != nil {
    return fmt.Errorf("failed to create directory %s: %w", destPath, err)
}

// Line 160
// Ensure parent directory exists with owner+group permissions only (0750)
if err := os.MkdirAll(filepath.Dir(destPath), 0750); err != nil {
    return fmt.Errorf("failed to create parent directory for %s: %w", destPath, err)
}

Security Best Practices Applied

  • Principle of Least Privilege: Directories now use owner+group permissions (0750) instead of world-readable (0755)
  • Defense in Depth: Restricts access to log artifacts at the filesystem level
  • Consistent Security Posture: Aligns with other security fixes in the codebase for file and directory permissions

Permission Breakdown

  • 0755 (old): rwxr-xr-x - Owner: read/write/execute, Group: read/execute, Others: read/execute
  • 0750 (new): rwxr-x--- - Owner: read/write/execute, Group: read/execute, Others: no access

Testing Considerations

  • ✅ Build completed successfully with go build ./pkg/cli/...
  • No functional changes to the log download/artifact flattening logic
  • Directories are still fully accessible by the owner and readable by the group
  • Backward compatible - more restrictive permissions do not break existing functionality

Impact

This is a security-only fix with no functional changes:

  • ✅ Log download and artifact flattening continue to work as expected
  • ✅ Owner and group access is preserved
  • ✅ Removes unnecessary world-readable access to log directories
  • ⚠️ Others no longer have read access to log directories (intentional security improvement)

Files Modified

  • pkg/cli/logs_download.go: Changed directory permissions from 0755 to 0750 (lines 153, 160)

🤖 Generated by gh-aw security fix agent
Triggered by: @pelikhan
Workflow Run: #20689975139

AI generated by Security Fix PR

…download.go

Fixes alerts #461 and #460 by using more restrictive directory permissions
(owner+group only) instead of world-readable permissions. This follows the
principle of least privilege for log artifact directories.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@pelikhan pelikhan marked this pull request as ready for review January 4, 2026 12:30
@pelikhan pelikhan merged commit ac9bb54 into main Jan 4, 2026
4 checks passed
@pelikhan pelikhan deleted the main-33656aa3b954d106 branch January 4, 2026 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant