Skip to content

Conversation

bharath-k1999
Copy link
Contributor

feat(security): Harden file sandboxing and path handling

This pull request improves the security and reliability of file sandboxing and path handling in Consul Template. It addresses potential vulnerabilities related to symlink and path confusion attacks by enforcing stricter path validation and adds comprehensive unit tests for edge cases.


Description

This change hardens the file sandbox functionality to prevent potential security vulnerabilities where a malicious template could escape the sandbox or disclose sensitive data. By normalizing, resolving, and using absolute paths for all checks and file operations, we mitigate the risk of symlink and path manipulation attacks.

The primary benefits are:

  • Stronger Security: Ensures sandbox boundaries are strictly enforced and mitigates risks of unauthorized file disclosure.
  • Better Reliability: Guarantees consistent path handling across all file operations and improves cross-platform compatibility.

There are no breaking changes to public APIs.


Key Changes

  • Sandbox Enforcement:

    • Improved pathInSandbox and fileFunc to use normalized, resolved, and absolute paths for all sandbox checks and file reads.
    • Prevents symlink and path manipulation attacks that could otherwise escape the sandbox.
  • Path Handling:

    • Removed unsafe path trimming from NewFileQuery to ensure the exact path checked is the one accessed.
    • Ensures all file operations use the same, fully-resolved path for both validation and reading.
  • Comprehensive Testing:

    • Added robust unit tests covering sandbox boundaries, symlink escapes, and files with leading/trailing spaces.
    • Tests automatically skip scenarios unsupported by the underlying filesystem (e.g., trailing spaces on macOS).

Notes

  • Filesystems like macOS APFS/HFS+ do not support trailing spaces in filenames; related tests are skipped on these platforms.

PCI review checklist

  • I have documented a clear reason for, and description of, the change I am making.

  • If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.

  • If applicable, I've documented the impact of any changes to security controls.

    Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.

@bharath-k1999 bharath-k1999 requested a review from a team as a code owner August 26, 2025 08:46
@bharath-k1999 bharath-k1999 self-assigned this Aug 27, 2025
@bharath-k1999 bharath-k1999 merged commit bca3e04 into hashicorp:main Sep 2, 2025
84 of 87 checks passed
return fmt.Errorf("'%s' is outside of sandbox", path)
}
if sandbox == "" {
return nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bharath-k1999 this will bypass the pathInSandbox if the sent sandbox is empty, not sure if it's the behaviour we want to have here. 🤔

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.

3 participants