atmos icon indicating copy to clipboard operation
atmos copied to clipboard
verified publisher icon cloudposse

fix: implement symlink security for vendor operations (CVE-2025-8959)

Open osterman opened this issue 6 months ago • 5 comments

what

  • Implement configurable symlink security policies for vendor operations
  • Add three security policies: allow_safe (default), reject_all, and allow_all
  • Validate symlinks stay within vendor boundaries to prevent directory traversal attacks
  • Create comprehensive PRD documenting the security implementation

why

  • CVE-2025-8959 (CVSS 7.5) is a critical vulnerability in HashiCorp's go-getter library that allows symlink attacks
  • Malicious repositories can use symlinks to gain unauthorized read access to files outside vendor boundaries
  • HashiCorp's fix (v1.7.9) simply disables all symlinks, breaking legitimate use cases
  • This implementation addresses the root cause while maintaining functionality for valid symlinks

references

  • CVE-2025-8959: https://nvd.nist.gov/vuln/detail/CVE-2025-8959
  • HashiCorp Security Advisory: https://discuss.hashicorp.com/t/hcsec-2025-23-hashicorp-go-getter-vulnerable-to-arbitrary-read-through-symlink-attack/76242
  • PRD: prd/vendor-symlink-security.prd.md

Summary

This PR implements a security feature that protects against symlink attacks in vendor operations without breaking existing workflows that rely on legitimate symlinks.

Key Changes

  1. Security Module (pkg/security/symlink_validator.go)

    • Three configurable policies: allow_safe, reject_all, allow_all
    • Boundary validation to prevent directory traversal
    • Integration with otiai10/copy library callbacks

  2. Configuration (vendor.policy.symlinks in atmos.yaml)

    vendor:
  policy:
    symlinks: "allow_safe"  # Default - blocks escaping symlinks

  3. Git Operations (pkg/downloader/git_getter.go)

    • Changed from removing all symlinks to validating based on policy
    • Maintains compatibility with go-getter v1.7.9

  4. Comprehensive Testing

    • Unit tests for symlink validation logic
    • Integration tests specifically for CVE-2025-8959 attack scenarios
    • Tests for all three security policies

  5. Documentation

    • Complete PRD following CloudPosse standards
    • Updated vendor-pull.mdx with configuration examples
    • Security best practices and troubleshooting guide

Security Impact

  • Default Protection: The default allow_safe policy immediately protects users from CVE-2025-8959
  • Backward Compatibility: Users can opt into allow_all for legacy behavior if needed
  • Maximum Security: reject_all available for high-security environments

Test Results

All tests pass, including specific tests for:

  • CVE-2025-8959 attack vectors (absolute paths, directory traversal, /etc/passwd)
  • Legitimate internal symlinks continue to work with allow_safe
  • Policy configuration and parsing
  • Cross-platform compatibility (Linux/macOS)

🤖 Generated with Claude Code

Co-Authored-By: Claude [email protected]

Summary by CodeRabbit

  • New Features

    • Configurable symlink security for vendoring with policies: allow_safe (default), reject_all, allow_all; set via atmos.yaml or ATMOS_VENDOR_POLICY_SYMLINKS. Vendor pulls and git downloads now respect the selected policy.

  • Documentation

    • Expanded vendor-pull docs with examples, best practices and CVE guidance.
    • Added PRD guidance and a vendor-symlink-security PRD.

  • Tests

    • Comprehensive tests covering symlink policies, attack scenarios, and integrations.

osterman avatar Sep 03 '25 15:09 osterman

[!WARNING]

This PR exceeds the recommended limit of 1,000 lines.

Large PRs are difficult to review and may be rejected due to their size.

Please verify that this PR does not address multiple issues. Consider refactoring it into smaller, more focused PRs to facilitate a smoother review process.

mergify[bot] avatar Sep 03 '25 15:09 mergify[bot]

[!WARNING]

Rate limit exceeded

@osterman has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 52 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 74cc4810a4df38c03f0cc1974bcc48d0218ea07a and 0c1212d453fb54a821a50d1882a96485a79a8ec2.

📒 Files selected for processing (20)
  • CLAUDE.md (2 hunks)
  • cmd/root.go (1 hunks)
  • internal/exec/copy_glob.go (3 hunks)
  • internal/exec/copy_glob_test.go (4 hunks)
  • internal/exec/stack_processor_utils.go (1 hunks)
  • internal/exec/vendor_component_utils.go (6 hunks)
  • internal/exec/vendor_model.go (3 hunks)
  • internal/exec/vendor_utils.go (0 hunks)
  • pkg/config/utils.go (2 hunks)
  • pkg/config/utils_test.go (2 hunks)
  • pkg/downloader/git_getter.go (1 hunks)
  • pkg/hooks/store_cmd.go (1 hunks)
  • pkg/schema/schema.go (1 hunks)
  • pkg/security/symlink_validator.go (1 hunks)
  • pkg/security/symlink_validator_test.go (1 hunks)
  • prd/README.md (1 hunks)
  • prd/vendor-symlink-security.prd.md (1 hunks)
  • tests/snapshots/TestCLICommands_atmos_describe_config.stdout.golden (1 hunks)
  • tests/vendor_symlink_security_test.go (1 hunks)
  • website/docs/cli/commands/vendor/vendor-pull.mdx (1 hunks)
📝 Walkthrough

Walkthrough

Adds a policy-driven symlink security subsystem, integrates policy-aware handling into vendoring and git download flows, threads atmosConfig through copy operations, updates schema/config/docs/PRDs, removes a legacy copy helper, and adds unit/integration tests and minor import alias changes.

Changes

Cohort / File(s) Summary
Documentation & PRDs
CLAUDE.md, prd/README.md, prd/vendor-symlink-security.prd.md, website/docs/cli/commands/vendor/vendor-pull.mdx		 Adds PRD guidance and a vendor-symlink-security PRD; expands vendor-pull docs with symlink policy configuration, examples, and security guidance.
Symlink Security Subsystem
pkg/security/symlink_validator.go, pkg/security/symlink_validator_test.go		 New SymlinkPolicy type and constants, ParsePolicy, IsSymlinkSafe, CreateSymlinkHandler, ValidateSymlinks, GetPolicyFromConfig, and extensive tests including CVE-style scenarios.
Schema & Config
pkg/schema/schema.go, pkg/config/utils.go, tests/snapshots/TestCLICommands_atmos_describe_config.stdout.golden		 Adds VendorPolicy and Vendor.Policy.Symlinks; maps ATMOS_VENDOR_POLICY_SYMLINKS into config and expands viper-based env processing; updates config snapshot.
Vendoring & Exec Integration
internal/exec/vendor_component_utils.go, internal/exec/vendor_model.go, internal/exec/copy_glob.go, internal/exec/copy_glob_test.go		 Threads atmosConfig through copy flows, replaces hard-coded cp.Deep with policy-derived OnSymlink via security.CreateSymlinkHandler, updates signatures and tests (tests pass nil).
Vendoring Utils Cleanup
internal/exec/vendor_utils.go		 Removes legacy copyToTarget helper and github.com/otiai10/copy usage; eliminates that copy path/URI sanitization.
Downloader / Git Getter
pkg/downloader/git_getter.go		 Adds Policy security.SymlinkPolicy to CustomGitGetter; validates symlinks with security.ValidateSymlinks (defaults to allow_safe); deprecates old removeSymlinks path.
Integration & Unit Tests
tests/vendor_symlink_security_test.go, pkg/security/symlink_validator_test.go		 New integration tests exercising allow_safe/reject_all/allow_all behaviors and CVE-like attack scenarios; unit tests for policy parsing and validation.
Import Alias Normalization
cmd/root.go, pkg/hooks/store_cmd.go, tests/cli_test.go		 Aliases github.com/charmbracelet/log as log to avoid stdlib conflict; no behavioral changes.
Errors & Stack Manifest Handling
errors/errors.go, internal/exec/stack_processor_utils.go		 Adds ErrInvalidStackManifest and wraps stack-manifest-related errors with that sentinel.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant CLI as CLI (vendor command)
  participant VM as VendorModel
  participant Exec as Exec / Copy
  participant Sec as Security Module
  participant CP as cp.Copy

  CLI->>VM: vendor pull/install (with atmosConfig)
  VM->>Sec: GetPolicyFromConfig(atmosConfig)
  Sec-->>VM: SymlinkPolicy
  VM->>Exec: downloadAndInstall(..., atmosConfig)
  Exec->>Sec: CreateSymlinkHandler(baseDir, policy)
  Sec-->>Exec: OnSymlink handler
  Exec->>CP: Copy(src, dst, Options{OnSymlink: handler})
  CP-->>Exec: result
  Exec-->>VM: done
  VM-->>CLI: completed

sequenceDiagram
  autonumber
  participant Caller as VendorSource (git)
  participant GG as CustomGitGetter
  participant Sec as Security Module
  participant FS as Filesystem

  Caller->>GG: Get(repo, dst)
  GG->>Sec: ValidateSymlinks(dst, GG.Policy|default)
  Sec->>FS: Walk & remove/allow symlinks per policy
  FS-->>Sec: done
  Sec-->>GG: ok / error
  GG-->>Caller: result

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

  • cloudposse/atmos#1024 — Modifies vendoring copy/URI handling and may overlap with removal/refactor of the legacy copy helper.
  • cloudposse/atmos#1494 — Touches stack processing/error-wrapping and the ErrInvalidStackManifest usage introduced here.
  • cloudposse/atmos#1163 — Changes environment-processing/config handling similar to the viper-based env changes in this PR.

Suggested reviewers

  • aknysh
  • mcalhoun
  • kevcube

Pre-merge checks and finishing touches and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 72.73% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly and accurately describes the primary change: implementing symlink security for vendor operations to mitigate CVE-2025-8959, which aligns with the code, tests, configuration, and docs in the changeset. It is specific, concise, and communicates the main intent to reviewers at a glance.

[!WARNING]

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • CVE-2025: Entity not found: Issue - Could not find referenced Issue.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Sep 03 '25 15:09 coderabbitai[bot]

📝 Walkthrough

Walkthrough

Introduces configurable symlink-handling policies and integrates them across vendoring, copy, and git download paths. Adds a new security module, schema support for policy, updates function signatures to pass config, replaces hardcoded symlink behavior with policy-driven handlers, and adds tests and docs (including PRDs and CLI docs) for the new behavior.

Changes

Cohort / File(s) Change summary
Security module
pkg/security/symlink_validator.go, pkg/security/symlink_validator_test.go		 New policy-based symlink validator: policy enum, parsing, safety checks, cp.OnSymlink handler, tree validation, config lookup; comprehensive unit tests including CVE scenarios.
Schema
pkg/schema/schema.go		 Adds VendorPolicy and Vendor.Policy.Symlinks to configure symlink handling.
Downloader (git getter)
pkg/downloader/git_getter.go		 Adds CustomGitGetter.Policy; switches from removing symlinks to policy-based validation after fetch; updates imports/comments; deprecates old removal helper.
Vendor integration
internal/exec/vendor_component_utils.go, internal/exec/vendor_model.go, internal/exec/vendor_utils.go		 Threads atmosConfig; replaces hardcoded cp.Deep with security.CreateSymlinkHandler(...); adds policy retrieval; adds file-skip func in component copy; updates copy targets to pass config.
Copy with globs
internal/exec/copy_glob.go, internal/exec/copy_glob_test.go		 Extends copyToTargetWithPatterns signature to accept atmosConfig; in no-pattern path uses policy-driven OnSymlink; tests updated to pass nil.
End-to-end tests
tests/vendor_symlink_security_test.go		 Adds integration tests validating policy behavior across copy/vendor flows and CVE-like attack blocking (skips on Windows).
Docs & PRDs
CLAUDE.md, prd/README.md, prd/vendor-symlink-security.prd.md, website/docs/cli/commands/vendor/vendor-pull.mdx		 Adds PRD workflow and dedicated vendor symlink security PRD; documents vendor.policy.symlinks options, behavior, examples, troubleshooting; note: doc block appears twice in vendor-pull.mdx.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer
  participant CFG as Atmos Config
  participant VND as Vendor Ops
  participant SEC as Security Module
  participant CP as cp.Copy / FS
  participant GIT as Git Getter

  Dev->>VND: vendor pull / copy
  VND->>CFG: Read vendor.policy.symlinks
  CFG-->>VND: symlink policy (string)
  VND->>SEC: Parse/GetPolicyFromConfig
  SEC-->>VND: SymlinkPolicy
  VND->>SEC: CreateSymlinkHandler(baseDir, policy)
  SEC-->>VND: OnSymlink handler
  VND->>CP: Copy(..., OnSymlink=handler, Skip=...)
  CP-->>VND: Copy result

  rect rgb(242,248,255)
    note right of GIT: New/changed
    VND->>GIT: Fetch source (git/https)
    GIT->>SEC: ValidateSymlinks(root, policy)
    SEC-->>GIT: ok/error (removes/keeps per policy)
  end

  VND-->>Dev: Completed vendoring

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60–90 minutes

Possibly related PRs

  • cloudposse/atmos#1419 — Also edits CLAUDE.md to introduce PRD content/workflow; overlaps with documentation changes here.
  • cloudposse/atmos#869 — Threads configuration/context through functions; related to passing atmosConfig for policy resolution.
  • cloudposse/atmos#984 — Touches vendoring and copy/glob code paths; intersects with symlink-handling integration.

Suggested labels

patch

Suggested reviewers

  • aknysh
  • milldr
  • osterman

[!WARNING]

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • CVE-2025: Entity not found: Issue - Could not find referenced Issue.
✨ Finishing Touches
  • [ ] 📝 Generate Docstrings
🧪 Generate unit tests
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment
  • [ ] Commit unit tests in branch secure-symlink-handling

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary or @auto-summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @auto-title anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot] avatar Sep 03 '25 16:09 coderabbitai[bot]

💥 This pull request now has conflicts. Could you fix it @osterman? 🙏

mergify[bot] avatar Sep 24 '25 01:09 mergify[bot]

💥 This pull request now has conflicts. Could you fix it @osterman? 🙏

mergify[bot] avatar Sep 27 '25 01:09 mergify[bot]