Skip to content

Secure file opening #48

@tamtom

Description

@tamtom

Summary

File upload commands (bundles, images, deobfuscation, sync import) currently open files directly without validating resolved paths. A malicious or misconfigured symlink could cause the CLI to read files outside the intended directory, creating a path traversal vulnerability. This issue adds a secure file-opening package that resolves symlinks and verifies the final path is within an expected directory boundary.

Implementation Plan

  • Create a secureopen package with a primary function: SecureOpen(path string, allowedDirs ...string) (*os.File, error)
  • Implementation steps within SecureOpen:
    1. Clean the path using filepath.Clean()
    2. Resolve all symlinks using filepath.EvalSymlinks()
    3. Convert to absolute path using filepath.Abs()
    4. Verify the resolved absolute path starts with one of the allowed directory prefixes
    5. If no allowedDirs provided, verify the resolved path is within the current working directory
    6. Open the file only after validation passes
  • Add a SecureOpenFile(path string, flag int, perm os.FileMode, allowedDirs ...string) (*os.File, error) variant for custom open flags
  • Return clear, descriptive error messages:
    • "file path resolves outside allowed directory: resolved %s, allowed %s"
    • "failed to resolve symlinks: %w"
  • Wire into all file-opening call sites:
    • bundles upload — bundle file
    • apks upload — APK file
    • images upload — image file
    • deobfuscation upload — mapping file
    • sync import — import file
    • auth login --service-account — service account key file
  • Allowed directory defaults: current working directory + explicit --file path's parent directory

Files to Create/Modify

  • Create: internal/secureopen/secureopen.go — Core secure file-opening logic
  • Create: internal/secureopen/secureopen_test.go — Comprehensive tests
  • Modify: internal/cli/bundles/upload.go — Use secureopen.SecureOpen()
  • Modify: internal/cli/apks/upload.go — Use secureopen.SecureOpen()
  • Modify: internal/cli/images/upload.go — Use secureopen.SecureOpen()
  • Modify: internal/cli/deobfuscation/upload.go — Use secureopen.SecureOpen()
  • Modify: Other commands that open user-specified files

Testing

  • Unit test: regular file opens successfully
  • Unit test: file within allowed directory opens successfully
  • Unit test: symlink within allowed directory resolves and opens
  • Unit test: symlink pointing outside allowed directory is rejected with clear error
  • Unit test: ../ traversal attempts are blocked
  • Unit test: absolute path outside allowed directory is rejected
  • Unit test: deeply nested symlink chains are fully resolved
  • Unit test: non-existent file returns appropriate error (not a security error)
  • Unit test: empty allowedDirs defaults to current working directory
  • Unit test: multiple allowedDirs — file in any one of them is accepted
  • Test on both macOS and Linux (symlink behavior differences)

Acceptance Criteria

  • secureopen.SecureOpen() resolves symlinks before opening files
  • Files outside allowed directories are rejected with a clear error message
  • Path traversal via ../ or symlinks is prevented
  • All upload commands use secureopen for file access
  • Regular (non-symlink) file operations continue to work without friction
  • Error messages are descriptive and actionable (not just "permission denied")
  • No new external dependencies
  • All existing tests pass

Metadata

Metadata

Assignees

No one assigned

    Labels

    asc-parityFeature parity with ASC CLIdxDeveloper experience improvement

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions