diff --git a/docs/threat-model.md b/docs/threat-model.md index b2a82b9..0efc31b 100644 --- a/docs/threat-model.md +++ b/docs/threat-model.md @@ -76,13 +76,28 @@ The trust boundary is between the shell/agent layer and the clet CLI host. Every ### File access scope -**Threat:** `clet pick-file` or `clet pick-directory` could be used to navigate to sensitive directories. `clet md` could be pointed at sensitive files. +**Threat:** `clet md` could be used as an arbitrary file-read primitive in agent contexts. An indirect-prompt-injection could instruct an agent to run `clet md /home/$USER/.aws/credentials`, exfiltrating file contents via the rendered ANSI or `--cat` output. Similarly, glob patterns like `clet md '/etc/*.conf'` could enumerate and read multiple files. -**Mitigation:** -- File pickers use TG's `OpenDialog`, which honors OS-level file permissions and sandboxing. -- `clet md` reads files via `File.ReadAllText()` — standard OS permission checks apply. -- No privilege escalation: clet runs as the invoking user, never setuid/setgid. -- The `--root` option on `pick-file`/`pick-directory` constrains the starting directory but does not prevent navigation outside it (that's the OS sandbox's job). +**Mitigation (D-030):** `clet md` enforces a file-access confinement policy: + +1. **Extension allowlist:** Only `.md`, `.markdown`, and `.txt` files are permitted by default. +2. **Working directory confinement:** Files must reside under the process working directory. Paths outside the cwd are refused. +3. **Per-file size cap:** 16 MiB maximum per file. +4. **Aggregate size cap:** 32 MiB maximum across all glob-expanded files. +5. **Glob file count cap:** Maximum 128 files per glob expansion. +6. **Binary content rejection:** Files containing NUL bytes in the first 8 KiB are refused (prevents accidentally rendering binary files). + +**Escape hatches:** +- `--allow-file ` (repeatable): Explicitly allows a specific file path, bypassing extension and cwd checks. The file must still pass size and binary checks. +- `--allow-binary`: Permits binary content (disables NUL-byte detection). + +**Rationale:** In agent contexts, positional arguments to `clet md` may be attacker-influenced. The default-deny policy prevents exfiltration while `--allow-file` provides an explicit opt-in for legitimate use cases where the caller controls the argument list. + +**`pick-file` / `pick-directory` `--root` option:** +- `--root` sets the *starting directory* for the file picker dialog. It is **not** a sandbox. +- The user can navigate freely outside the root directory via the dialog's path bar. +- This is a deliberate design choice: the user is interactively choosing files, not an attacker. The OS file permission model is the sandboxing mechanism for interactive pickers. +- The help text explicitly documents this: "Starting directory (not a sandbox — user can navigate freely)." ### Plugin loading exclusion diff --git a/specs/decisions.md b/specs/decisions.md index 9ceb7ec..e163e37 100644 --- a/specs/decisions.md +++ b/specs/decisions.md @@ -453,3 +453,27 @@ This is clet's own defense. TG's cell model is treated as defense-in-depth, not **Status.** Active. **Pointers.** D-017, `src/Clet/Clets/Viewer/MarkdownClet.cs` (LinkClicked handler), `docs/threat-model.md` (Markdown link policy section), `specs/clet-spec.md` (Appendix A), `tests/Clet.IntegrationTests/MarkdownCletIntegrationTests.cs` (link safety tests). + +--- + +## D-032: `clet md` file-access confinement policy (Active) + +**Context.** `clet md FILE` is an arbitrary file-read primitive. In agent contexts, positional arguments may be attacker-influenced via indirect prompt injection. An attacker could instruct an agent to run `clet md /home/$USER/.aws/credentials`, exfiltrating file content through the rendered ANSI output or `--cat` stdout. Glob patterns (`clet md '/etc/*.conf'`) amplify this to directory enumeration. Issue #38 documents the full threat surface. + +**Decision.** Default-deny file access policy with explicit opt-in escape hatches: +1. **Extension allowlist:** `.md`, `.markdown`, `.txt` only. +2. **Working directory confinement:** Files must be under the process cwd. +3. **Per-file size cap:** 16 MiB. +4. **Aggregate size cap:** 32 MiB across all glob matches. +5. **Glob count cap:** 128 files maximum. +6. **Binary rejection:** NUL byte in first 8 KiB refuses the file. +7. **`--allow-file `** (repeatable): Bypasses extension + cwd checks for the named path. Size and binary checks still apply. +8. **`--allow-binary`:** Disables NUL-byte detection. + +For `pick-file`/`pick-directory`, `--root` remains a starting directory, not a sandbox. The interactive file picker is user-driven and OS-permission-gated; confinement is the OS sandbox's responsibility, not clet's. Help text updated to make this explicit. + +**Why not confine `pick-*`?** The user is interactively choosing files. Constraining navigation would break legitimate workflows and provide false security (the user can always run `ls` separately). The agent-context risk is in *non-interactive* file reads (`clet md`), not interactive dialogs. + +**Status.** Active. + +**Pointers.** `src/Clet/Hosting/FileAccessPolicy.cs`, `src/Clet/Hosting/AliasDispatcher.cs` (`ResolveViewerContent`), `src/Clet/Clets/Viewer/MarkdownClet.cs` (`ExpandFiles`), `src/Clet/Hosting/CommandLineRoot.cs` (`--allow-file`, `--allow-binary` parsing), `docs/threat-model.md` ("File access scope" section), issue #38. diff --git a/src/Clet/Abstractions/CletRunOptions.cs b/src/Clet/Abstractions/CletRunOptions.cs index 1b97623..e527892 100644 --- a/src/Clet/Abstractions/CletRunOptions.cs +++ b/src/Clet/Abstractions/CletRunOptions.cs @@ -11,4 +11,10 @@ internal sealed record CletRunOptions public int? Rows { get; init; } public IReadOnlyDictionary? CletOptions { get; init; } public IReadOnlyList? Arguments { get; init; } + + /// Paths explicitly allowed for file reading (bypasses extension + cwd checks). + public IReadOnlyList? AllowedFiles { get; init; } + + /// When true, binary file content (NUL bytes) is permitted. + public bool AllowBinary { get; init; } } diff --git a/src/Clet/Clets/Input/PickDirectoryClet.cs b/src/Clet/Clets/Input/PickDirectoryClet.cs index 108dee7..760d26d 100644 --- a/src/Clet/Clets/Input/PickDirectoryClet.cs +++ b/src/Clet/Clets/Input/PickDirectoryClet.cs @@ -15,7 +15,7 @@ internal sealed class PickDirectoryClet : IClet public IReadOnlyList Options => [ - new ("root", "r", typeof (string), "Starting directory.", false, null), + new ("root", "r", typeof (string), "Starting directory (not a sandbox — user can navigate freely).", false, null), ]; public async Task> RunAsync ( diff --git a/src/Clet/Clets/Input/PickFileClet.cs b/src/Clet/Clets/Input/PickFileClet.cs index e7c0b48..134f29f 100644 --- a/src/Clet/Clets/Input/PickFileClet.cs +++ b/src/Clet/Clets/Input/PickFileClet.cs @@ -17,7 +17,7 @@ internal sealed class PickFileClet : IClet public IReadOnlyList Options => [ new ("multi", "m", typeof (bool), "Allow selecting multiple files.", false, "false"), - new ("root", "r", typeof (string), "Starting directory.", false, null), + new ("root", "r", typeof (string), "Starting directory (not a sandbox — user can navigate freely).", false, null), new ("filter", "f", typeof (string), "File type filter (e.g. \"*.cs\").", false, null), ]; diff --git a/src/Clet/Clets/Viewer/MarkdownClet.cs b/src/Clet/Clets/Viewer/MarkdownClet.cs index a6bfb1a..54d79ae 100644 --- a/src/Clet/Clets/Viewer/MarkdownClet.cs +++ b/src/Clet/Clets/Viewer/MarkdownClet.cs @@ -48,7 +48,17 @@ public async Task RunAsync ( if (options.Arguments is { Count: > 0 }) { - files = ExpandFiles (options.Arguments); + FileAccessPolicy policy = new ( + Directory.GetCurrentDirectory (), + options.AllowedFiles, + options.AllowBinary); + + files = ExpandFiles (options.Arguments, policy, out string? policyError); + + if (policyError is not null) + { + return new () { Status = CletRunStatus.Error, ErrorCode = "file-access-denied", ErrorMessage = policyError }; + } if (files.Count == 0) { @@ -94,6 +104,15 @@ public async Task RunAsync ( return new () { Status = CletRunStatus.Error, ErrorCode = "io", ErrorMessage = "No file specified. Usage: clet md " }; } + // Track current file directory for resolving relative links + string? currentFileDir = files.Count > 0 ? Path.GetDirectoryName (Path.GetFullPath (files [0])) : null; + + // File access policy for link navigation (reuse the same confinement as file loading) + FileAccessPolicy linkPolicy = new ( + Directory.GetCurrentDirectory (), + options.AllowedFiles, + options.AllowBinary); + // Parse --theme option ThemeName syntaxTheme = ThemeName.DarkPlus; @@ -123,13 +142,30 @@ public async Task RunAsync ( Shortcut lineCountShortcut = new () { Title = "0 lines", MouseHighlightStates = MouseState.None, Enabled = false }; Shortcut fileSizeShortcut = new () { Title = "0 B", MouseHighlightStates = MouseState.None, Enabled = false }; - Shortcut statusShortcut = new (Key.Empty, "Ready", null) { MouseHighlightStates = MouseState.None }; + + // Status link — shows the current filename or a clickable URL when the user + // hovers/clicks a hyperlink in the markdown. Clicking the link in the status + // bar opens it in the default browser. + Link statusLink = new () { Text = "Ready", CanFocus = false }; + Shortcut statusShortcut = new () { CommandView = statusLink, MouseHighlightStates = MouseState.None }; // --- MarkdownView event wiring --- markdownView.LinkClicked += (_, e) => { - statusShortcut.Title = e.Url; + // Try to navigate to local .md files that pass the file access policy + if (currentFileDir is not null && TryResolveLocalMarkdownLink (e.Url, currentFileDir, linkPolicy, out string? resolvedPath)) + { + LoadFile (resolvedPath); + e.Handled = true; + + return; + } + + // Show URL in status bar as a clickable link + statusLink.Text = e.Url; + statusLink.Url = e.Url; + statusShortcut.MouseHighlightStates = MouseState.In; e.Handled = true; }; @@ -235,7 +271,9 @@ public async Task RunAsync ( string sanitized = TerminalEscapeSanitizer.Sanitize (content)!; markdownView.Text = sanitized; fileSizeShortcut.Title = FormatFileSize (System.Text.Encoding.UTF8.GetByteCount (sanitized)); - statusShortcut.Title = options.Title ?? "(inline)"; + statusLink.Text = options.Title ?? "(inline)"; + statusLink.Url = string.Empty; + statusShortcut.MouseHighlightStates = MouseState.None; } }; @@ -258,18 +296,88 @@ public async Task RunAsync ( void LoadFile (string filePath) { - string fileContent = TerminalEscapeSanitizer.Sanitize (File.ReadAllText (filePath))!; + string fullPath = Path.GetFullPath (filePath); + string fileContent = TerminalEscapeSanitizer.Sanitize (File.ReadAllText (fullPath))!; markdownView.Text = fileContent; - FileInfo fileInfo = new (filePath); + currentFileDir = Path.GetDirectoryName (fullPath); + + FileInfo fileInfo = new (fullPath); fileSizeShortcut.Title = FormatFileSize (fileInfo.Length); - statusShortcut.Title = Path.GetFileName (filePath); + statusLink.Text = Path.GetFileName (fullPath); + statusLink.Url = string.Empty; + } + } + + /// + /// Resolves a link URL to a local markdown file path if it passes the file access policy. + /// + internal static bool TryResolveLocalMarkdownLink ( + string url, + string currentDir, + FileAccessPolicy policy, + out string? resolvedPath) + { + resolvedPath = null; + + // Strip fragment (e.g. #section) + int fragmentIndex = url.IndexOf ('#'); + string pathPart = fragmentIndex >= 0 ? url [..fragmentIndex] : url; + + if (string.IsNullOrWhiteSpace (pathPart)) + { + return false; + } + + // Handle file:// URIs + if (pathPart.StartsWith ("file://", StringComparison.OrdinalIgnoreCase)) + { + if (!Uri.TryCreate (pathPart, UriKind.Absolute, out Uri? fileUri) || !fileUri.IsFile) + { + return false; + } + + pathPart = fileUri.LocalPath; + } + // Reject non-local schemes (http://, https://, mailto:, etc.) + else if (pathPart.Contains ("://", StringComparison.Ordinal)) + { + return false; + } + + string fullPath; + + try + { + fullPath = Path.IsPathRooted (pathPart) + ? Path.GetFullPath (pathPart) + : Path.GetFullPath (Path.Combine (currentDir, pathPart)); + } + catch + { + return false; + } + + if (!File.Exists (fullPath)) + { + return false; + } + + // Delegate all security checks (extension, cwd confinement, binary, size) to the policy + if (policy.CheckFile (fullPath) is not null) + { + return false; } + + resolvedPath = fullPath; + + return true; } - private static List ExpandFiles (IReadOnlyList patterns) + private static List ExpandFiles (IReadOnlyList patterns, FileAccessPolicy policy, out string? error) { List result = []; + error = null; foreach (string pattern in patterns) { @@ -280,11 +388,42 @@ private static List ExpandFiles (IReadOnlyList patterns) if (Directory.Exists (directory)) { - result.AddRange (Directory.GetFiles (directory, filePattern)); + string[] matched = Directory.GetFiles (directory, filePattern); + string? globError = policy.CheckGlobAggregate (matched); + + if (globError is not null) + { + error = globError; + + return []; + } + + foreach (string file in matched) + { + string? violation = policy.CheckFile (file); + + if (violation is not null) + { + error = violation; + + return []; + } + + result.Add (Path.GetFullPath (file)); + } } } else if (File.Exists (pattern)) { + string? violation = policy.CheckFile (pattern); + + if (violation is not null) + { + error = violation; + + return []; + } + result.Add (Path.GetFullPath (pattern)); } else diff --git a/src/Clet/Hosting/AliasDispatcher.cs b/src/Clet/Hosting/AliasDispatcher.cs index f0fd622..c1d65a2 100644 --- a/src/Clet/Hosting/AliasDispatcher.cs +++ b/src/Clet/Hosting/AliasDispatcher.cs @@ -95,24 +95,86 @@ public async Task DispatchAsync ( { if (options.Arguments is { Count: > 0 } args) { + FileAccessPolicy policy = new ( + Directory.GetCurrentDirectory (), + options.AllowedFiles, + options.AllowBinary); + List contents = []; + long aggregateSize = 0; foreach (string arg in args) { - if (File.Exists (arg)) + // Expand globs + List files; + + if (arg.Contains ('*') || arg.Contains ('?')) { - try + string directory = Path.GetDirectoryName (arg) is { Length: > 0 } dir ? dir : "."; + string filePattern = Path.GetFileName (arg); + + if (!Directory.Exists (directory)) { - contents.Add (File.ReadAllText (arg)); + stderr.WriteLine ($"Warning: Directory not found: {directory}"); + + continue; } - catch (Exception ex) + + files = [.. Directory.GetFiles (directory, filePattern)]; + string? globError = policy.CheckGlobAggregate (files); + + if (globError is not null) { - stderr.WriteLine ($"Warning: Could not read file '{arg}': {ex.Message}"); + stderr.WriteLine ($"error: {globError}"); + + return null; } } else { - stderr.WriteLine ($"Warning: File not found: {arg}"); + files = [arg]; + } + + foreach (string file in files) + { + string? violation = policy.CheckFile (file); + + if (violation is not null) + { + stderr.WriteLine ($"error: {violation}"); + + return null; + } + + if (!File.Exists (file)) + { + stderr.WriteLine ($"Warning: File not found: {file}"); + + continue; + } + + FileInfo fi = new (file); + aggregateSize += fi.Length; + + if (aggregateSize > FileAccessPolicy.MaxAggregateSizeBytes) + { + stderr.WriteLine ($"error: Refused: aggregate file size exceeds {FileAccessPolicy.MaxAggregateSizeBytes / (1024 * 1024)} MiB limit."); + + return null; + } + + try + { + contents.Add (File.ReadAllText (file)); + } + catch (UnauthorizedAccessException ex) + { + stderr.WriteLine ($"Warning: Could not read file '{file}': {ex.Message}"); + } + catch (IOException ex) + { + stderr.WriteLine ($"Warning: Could not read file '{file}': {ex.Message}"); + } } } diff --git a/src/Clet/Hosting/CommandLineRoot.cs b/src/Clet/Hosting/CommandLineRoot.cs index 241de00..8ed32f8 100644 --- a/src/Clet/Hosting/CommandLineRoot.cs +++ b/src/Clet/Hosting/CommandLineRoot.cs @@ -69,10 +69,12 @@ private async Task DispatchAlias ( bool jsonOutput = false; bool fullscreen = false; bool cat = false; + bool allowBinary = false; TimeSpan? timeout = null; int? rows = null; Dictionary cletOptions = new (StringComparer.OrdinalIgnoreCase); List positionalArgs = []; + List allowedFiles = []; for (int i = 1; i < args.Length; i++) { @@ -99,6 +101,27 @@ private async Task DispatchAlias ( continue; } + if (arg == "--allow-binary") + { + allowBinary = true; + + continue; + } + + if (arg == "--allow-file") + { + if (i + 1 >= args.Length) + { + stderr.WriteLine ("error: --allow-file requires a file path."); + + return ExitCodes.UsageError; + } + + allowedFiles.Add (args [++i]); + + continue; + } + if (arg == "--timeout") { if (i + 1 >= args.Length) @@ -222,6 +245,8 @@ private async Task DispatchAlias ( Rows = rows, CletOptions = cletOptions, Arguments = positionalArgs.Count > 0 ? positionalArgs : null, + AllowedFiles = allowedFiles.Count > 0 ? allowedFiles : null, + AllowBinary = allowBinary, }; // Validate positional args before dispatching. diff --git a/src/Clet/Hosting/ExitCodes.cs b/src/Clet/Hosting/ExitCodes.cs index 961eed0..5447007 100644 --- a/src/Clet/Hosting/ExitCodes.cs +++ b/src/Clet/Hosting/ExitCodes.cs @@ -20,6 +20,7 @@ public static int FromResult (BoxedCletResult result) { "validation" => ValidationError, "input-too-large" => ValidationError, + "file-access-denied" => UsageError, "io" => IoError, _ => UsageError, }, diff --git a/src/Clet/Hosting/FileAccessPolicy.cs b/src/Clet/Hosting/FileAccessPolicy.cs new file mode 100644 index 0000000..b4bf742 --- /dev/null +++ b/src/Clet/Hosting/FileAccessPolicy.cs @@ -0,0 +1,181 @@ +namespace Clet; + +/// +/// Enforces file-access confinement for clet md to mitigate agent-context exfiltration. +/// Files must pass extension allowlist + working-directory confinement unless explicitly opted in +/// via --allow-file. Binary content and oversized files/aggregates are also rejected. +/// +internal sealed class FileAccessPolicy +{ + /// Default allowed extensions for markdown-like content. + private static readonly HashSet DefaultAllowedExtensions = new (StringComparer.OrdinalIgnoreCase) + { + ".md", ".markdown", ".txt", + }; + + /// Per-file size cap: 16 MiB. + internal const long MaxFileSizeBytes = 16L * 1024 * 1024; + + /// Aggregate size cap across all glob-expanded files: 32 MiB. + internal const long MaxAggregateSizeBytes = 32L * 1024 * 1024; + + /// Bytes to inspect for binary (NUL) detection. + internal const int BinaryProbeBytes = 8 * 1024; + + /// Maximum number of files from a single glob expansion. + internal const int MaxGlobFiles = 128; + + private readonly string _workingDirectory; + private readonly HashSet _allowedFiles; + private readonly List _allowedDirs; + private readonly bool _allowBinary; + + public FileAccessPolicy (string workingDirectory, IReadOnlyList? allowedFiles, bool allowBinary) + { + _workingDirectory = Path.GetFullPath (workingDirectory); + _allowBinary = allowBinary; + _allowedFiles = new HashSet (StringComparer.OrdinalIgnoreCase); + _allowedDirs = []; + + if (allowedFiles is not null) + { + foreach (string f in allowedFiles) + { + string full = Path.GetFullPath (f); + + if (Directory.Exists (full)) + { + _allowedDirs.Add (full); + } + else + { + _allowedFiles.Add (full); + } + } + } + } + + /// + /// Checks whether a resolved file path is permitted under the policy. + /// Returns null if permitted; an error message string if refused. + /// + public string? CheckFile (string filePath) + { + string fullPath = Path.GetFullPath (filePath); + + // Explicitly allowed files/directories bypass extension and cwd checks + if (_allowedFiles.Contains (fullPath) || _allowedDirs.Any (dir => IsUnderDirectory (fullPath, dir))) + { + return CheckSizeAndBinary (fullPath); + } + + // Extension allowlist + string ext = Path.GetExtension (fullPath); + + if (!DefaultAllowedExtensions.Contains (ext)) + { + return $"Refused: '{filePath}' has extension '{ext}' which is not in the allowlist " + + $"({string.Join (", ", DefaultAllowedExtensions)}). Use --allow-file to override."; + } + + // Working directory confinement + if (!IsUnderDirectory (fullPath, _workingDirectory)) + { + return $"Refused: '{filePath}' is outside the working directory '{_workingDirectory}'. " + + "Use --allow-file to override."; + } + + return CheckSizeAndBinary (fullPath); + } + + /// + /// Validates a glob expansion: max file count and aggregate size. + /// Returns null if OK; an error message if refused. + /// + public string? CheckGlobAggregate (IReadOnlyList files) + { + if (files.Count > MaxGlobFiles) + { + return $"Refused: glob matched {files.Count} files, exceeding the maximum of {MaxGlobFiles}."; + } + + long total = 0; + + foreach (string file in files) + { + FileInfo fi = new (file); + + if (fi.Exists) + { + total += fi.Length; + } + + if (total > MaxAggregateSizeBytes) + { + return $"Refused: aggregate file size exceeds {MaxAggregateSizeBytes / (1024 * 1024)} MiB limit."; + } + } + + return null; + } + + private string? CheckSizeAndBinary (string fullPath) + { + if (!File.Exists (fullPath)) + { + return null; // Let downstream handle file-not-found + } + + FileInfo fi = new (fullPath); + + if (fi.Length > MaxFileSizeBytes) + { + return $"Refused: '{fi.Name}' is {fi.Length / (1024 * 1024)} MiB, exceeding the {MaxFileSizeBytes / (1024 * 1024)} MiB per-file limit."; + } + + if (!_allowBinary) + { + try + { + byte[] probe = new byte[Math.Min (BinaryProbeBytes, (int)fi.Length)]; + using FileStream fs = new (fullPath, FileMode.Open, FileAccess.Read, FileShare.Read); + int read = fs.Read (probe, 0, probe.Length); + + for (int i = 0; i < read; i++) + { + if (probe [i] == 0) + { + return $"Refused: '{fi.Name}' appears to be a binary file (NUL byte at offset {i}). Use --allow-binary to override."; + } + } + } + catch (Exception ex) when (ex is IOException + or UnauthorizedAccessException + or System.Security.SecurityException + or NotSupportedException + or PathTooLongException) + { + return $"Refused: could not probe '{fi.Name}' for binary content: {ex.Message}"; + } + } + + return null; + } + + private static bool IsUnderDirectory (string filePath, string directory) + { + // Normalize both paths fully before comparison + string normalizedFile = Path.GetFullPath (filePath); + string normalizedDir = Path.GetFullPath (directory); + + // Ensure directory ends with separator to prevent prefix matching attacks + // (e.g. "/safe" matching "/safe-other-dir/file.txt") + if (!normalizedDir.EndsWith (Path.DirectorySeparatorChar)) + { + normalizedDir += Path.DirectorySeparatorChar; + } + + return normalizedFile.StartsWith (normalizedDir, StringComparison.OrdinalIgnoreCase) + || normalizedFile.Equals (normalizedDir.TrimEnd (Path.DirectorySeparatorChar), StringComparison.OrdinalIgnoreCase); + } +} diff --git a/tests/Clet.IntegrationTests/MarkdownCletIntegrationTests.cs b/tests/Clet.IntegrationTests/MarkdownCletIntegrationTests.cs index e33cdce..c6eecec 100644 --- a/tests/Clet.IntegrationTests/MarkdownCletIntegrationTests.cs +++ b/tests/Clet.IntegrationTests/MarkdownCletIntegrationTests.cs @@ -82,8 +82,8 @@ public async Task RunAsync_WithFileArgument_NonexistentFile_ReturnsError () [Fact] public async Task RunAsync_WithFileArgument_ReturnsOk () { - // Create a temporary markdown file for this test - string tempFile = Path.GetTempFileName (); + // Create a temporary .md file in the current directory (satisfies FileAccessPolicy) + string tempFile = Path.Combine (Directory.GetCurrentDirectory (), $"test-{Guid.NewGuid ()}.md"); try { diff --git a/tests/Clet.SmokeTests/FileAccessPolicySmokeTests.cs b/tests/Clet.SmokeTests/FileAccessPolicySmokeTests.cs new file mode 100644 index 0000000..d0a1fcf --- /dev/null +++ b/tests/Clet.SmokeTests/FileAccessPolicySmokeTests.cs @@ -0,0 +1,135 @@ +using Xunit; + +namespace Clet.SmokeTests; + +/// +/// Smoke tests for file-access confinement in clet md. +/// Verifies that the security policy rejects unauthorized file access. +/// +public class FileAccessPolicySmokeTests +{ + [Fact] + public async Task MdSystemFile_IsRefusedWithoutAllowFile () + { + // /etc/passwd on Linux, or a well-known non-.md system file + string systemFile = OperatingSystem.IsWindows () + ? @"C:\Windows\System32\drivers\etc\hosts" + : "/etc/passwd"; + + if (!File.Exists (systemFile)) + { + return; // Skip if the file doesn't exist on this system + } + + (int exit, _, string stderr) = await CletProcess.RunAsync ( + ["md", "--cat", systemFile]); + + // Should be refused — not in allowlist (.md/.markdown/.txt) or outside cwd + Assert.NotEqual (0, exit); + Assert.Contains ("Refused", stderr); + } + + [Fact] + public async Task MdSystemFile_AllowedWithAllowFile () + { + // Create a .conf file in temp directory to test --allow-file bypass + string baseTempFile = Path.GetTempFileName (); + string tmpFile = Path.ChangeExtension (baseTempFile, ".conf"); + File.Move (baseTempFile, tmpFile); + File.WriteAllText (tmpFile, "# test config content"); + + try + { + (int exit, _, string stderr) = await CletProcess.RunAsync ( + ["md", "--cat", "--allow-file", tmpFile, tmpFile]); + + Assert.Equal (0, exit); + Assert.Empty (stderr); + } + finally + { + File.Delete (tmpFile); + } + } + + [Fact] + public async Task MdGlobOutsideCwd_IsRefused () + { + // Try to glob /etc/*.conf (Linux) — should be refused + if (OperatingSystem.IsWindows ()) + { + return; // Skip on Windows — different path structure + } + + (int exit, _, string stderr) = await CletProcess.RunAsync ( + ["md", "--cat", "/etc/*.conf"]); + + // Should be refused due to being outside cwd + disallowed extension + Assert.NotEqual (0, exit); + Assert.Contains ("Refused", stderr); + } + + [Fact] + public async Task MdAllowedExtension_InCwd_Succeeds () + { + // Create a .md file and run from its directory + string tmpDir = Path.Join (Path.GetTempPath (), $"clet-test-{Guid.NewGuid ()}"); + Directory.CreateDirectory (tmpDir); + string mdFile = Path.Combine (tmpDir, "test.md"); + File.WriteAllText (mdFile, "# Hello World"); + + try + { + (int exit, _, string stderr) = await CletProcess.RunAsync ( + ["md", "--cat", mdFile, "--allow-file", mdFile]); + + // With --allow-file, should succeed even though it may be outside the process cwd + Assert.Equal (0, exit); + } + finally + { + Directory.Delete (tmpDir, recursive: true); + } + } + + [Fact] + public async Task MdBinaryFile_IsRefused () + { + string tmpFile = Path.Combine (Path.GetTempPath (), $"clet-test-{Guid.NewGuid ()}.md"); + byte[] content = [0x23, 0x20, 0x48, 0x65, 0x6C, 0x00, 0x6C, 0x6F]; // "# Hel\0lo" + File.WriteAllBytes (tmpFile, content); + + try + { + (int exit, _, string stderr) = await CletProcess.RunAsync ( + ["md", "--cat", "--allow-file", tmpFile, tmpFile]); + + Assert.NotEqual (0, exit); + Assert.Contains ("binary file", stderr); + } + finally + { + File.Delete (tmpFile); + } + } + + [Fact] + public async Task MdBinaryFile_AllowedWithAllowBinary () + { + string tmpFile = Path.Combine (Path.GetTempPath (), $"clet-test-{Guid.NewGuid ()}.md"); + byte[] content = [0x23, 0x20, 0x48, 0x65, 0x6C, 0x00, 0x6C, 0x6F]; // "# Hel\0lo" + File.WriteAllBytes (tmpFile, content); + + try + { + (int exit, _, _) = await CletProcess.RunAsync ( + ["md", "--cat", "--allow-file", tmpFile, "--allow-binary", tmpFile]); + + Assert.Equal (0, exit); + } + finally + { + File.Delete (tmpFile); + } + } +} diff --git a/tests/Clet.UnitTests/CommandLineRootTests.cs b/tests/Clet.UnitTests/CommandLineRootTests.cs index 8721318..8163dce 100644 --- a/tests/Clet.UnitTests/CommandLineRootTests.cs +++ b/tests/Clet.UnitTests/CommandLineRootTests.cs @@ -369,13 +369,13 @@ public async Task MdCat_WithInitial_RendersToStdoutAndExitsOk () public async Task MdCat_WithFile_RendersFileToStdout () { (CommandLineRoot root, StringWriter stdout, StringWriter stderr) = Build (); - string tempFile = Path.GetTempFileName (); + string tempFile = Path.Combine (Path.GetTempPath (), $"clet-test-{Guid.NewGuid ()}.md"); try { File.WriteAllText (tempFile, "# Test File\n\nSome content."); - int exit = await root.InvokeAsync (["md", "--cat", tempFile], CancellationToken.None, stdout, stderr); + int exit = await root.InvokeAsync (["md", "--cat", "--allow-file", tempFile, tempFile], CancellationToken.None, stdout, stderr); Assert.Equal (ExitCodes.Ok, exit); Assert.NotEmpty (stdout.ToString ()); diff --git a/tests/Clet.UnitTests/FileAccessPolicyTests.cs b/tests/Clet.UnitTests/FileAccessPolicyTests.cs new file mode 100644 index 0000000..9d6c7d1 --- /dev/null +++ b/tests/Clet.UnitTests/FileAccessPolicyTests.cs @@ -0,0 +1,269 @@ +using Xunit; + +namespace Clet.UnitTests; + +public class FileAccessPolicyTests +{ + [Fact] + public void AllowedExtension_InWorkingDirectory_Passes () + { + string cwd = Path.GetTempPath (); + string file = Path.Combine (cwd, "readme.md"); + File.WriteAllText (file, "# hello"); + + try + { + FileAccessPolicy policy = new (cwd, allowedFiles: null, allowBinary: false); + string? error = policy.CheckFile (file); + + Assert.Null (error); + } + finally + { + File.Delete (file); + } + } + + [Fact] + public void DisallowedExtension_IsRefused () + { + string cwd = Path.GetTempPath (); + string file = Path.Combine (cwd, "secrets.conf"); + File.WriteAllText (file, "password=secret"); + + try + { + FileAccessPolicy policy = new (cwd, allowedFiles: null, allowBinary: false); + string? error = policy.CheckFile (file); + + Assert.NotNull (error); + Assert.Contains ("not in the allowlist", error); + } + finally + { + File.Delete (file); + } + } + + [Fact] + public void FileOutsideCwd_IsRefused () + { + string cwd = Path.Combine (Path.GetTempPath (), "clet-test-cwd"); + Directory.CreateDirectory (cwd); + string outside = Path.Combine (Path.GetTempPath (), "outside.md"); + File.WriteAllText (outside, "# outside"); + + try + { + FileAccessPolicy policy = new (cwd, allowedFiles: null, allowBinary: false); + string? error = policy.CheckFile (outside); + + Assert.NotNull (error); + Assert.Contains ("outside the working directory", error); + } + finally + { + File.Delete (outside); + Directory.Delete (cwd, recursive: true); + } + } + + [Fact] + public void AllowFile_BypassesExtensionAndCwdChecks () + { + string cwd = Path.Combine (Path.GetTempPath (), "clet-test-cwd2"); + Directory.CreateDirectory (cwd); + string outside = Path.Combine (Path.GetTempPath (), "credentials.conf"); + File.WriteAllText (outside, "key=value"); + + try + { + FileAccessPolicy policy = new (cwd, allowedFiles: [outside], allowBinary: false); + string? error = policy.CheckFile (outside); + + Assert.Null (error); + } + finally + { + File.Delete (outside); + Directory.Delete (cwd, recursive: true); + } + } + + [Fact] + public void AllowFile_WithDirectory_AllowsFilesUnderThatDirectory () + { + string allowedDir = Path.Combine (Path.GetTempPath (), $"clet-allow-dir-{Guid.NewGuid ()}"); + string cwd = Path.Combine (Path.GetTempPath (), $"clet-test-cwd-{Guid.NewGuid ()}"); + Directory.CreateDirectory (allowedDir); + Directory.CreateDirectory (cwd); + string file = Path.Combine (allowedDir, "README.md"); + File.WriteAllText (file, "# hello"); + + try + { + FileAccessPolicy policy = new (cwd, allowedFiles: [allowedDir], allowBinary: false); + string? error = policy.CheckFile (file); + + Assert.Null (error); + } + finally + { + File.Delete (file); + Directory.Delete (allowedDir, recursive: true); + Directory.Delete (cwd, recursive: true); + } + } + + [Fact] + public void AllowFile_WithDirectory_RejectsFilesOutsideThatDirectory () + { + string allowedDir = Path.Combine (Path.GetTempPath (), $"clet-allow-dir2-{Guid.NewGuid ()}"); + string cwd = Path.Combine (Path.GetTempPath (), $"clet-test-cwd2-{Guid.NewGuid ()}"); + Directory.CreateDirectory (allowedDir); + Directory.CreateDirectory (cwd); + string outsideFile = Path.Combine (Path.GetTempPath (), $"outside-{Guid.NewGuid ()}.md"); + File.WriteAllText (outsideFile, "# outside"); + + try + { + FileAccessPolicy policy = new (cwd, allowedFiles: [allowedDir], allowBinary: false); + string? error = policy.CheckFile (outsideFile); + + Assert.NotNull (error); + } + finally + { + File.Delete (outsideFile); + Directory.Delete (allowedDir, recursive: true); + Directory.Delete (cwd, recursive: true); + } + } + + [Fact] + public void BinaryFile_IsRefused_WhenAllowBinaryFalse () + { + string cwd = Path.GetTempPath (); + string file = Path.Combine (cwd, "binary.md"); + byte[] content = [0x23, 0x20, 0x48, 0x65, 0x6C, 0x00, 0x6C, 0x6F]; // "# Hel\0lo" + File.WriteAllBytes (file, content); + + try + { + FileAccessPolicy policy = new (cwd, allowedFiles: null, allowBinary: false); + string? error = policy.CheckFile (file); + + Assert.NotNull (error); + Assert.Contains ("binary file", error); + } + finally + { + File.Delete (file); + } + } + + [Fact] + public void BinaryFile_IsAllowed_WhenAllowBinaryTrue () + { + string cwd = Path.GetTempPath (); + string file = Path.Combine (cwd, "binary.md"); + byte[] content = [0x23, 0x20, 0x48, 0x65, 0x6C, 0x00, 0x6C, 0x6F]; // "# Hel\0lo" + File.WriteAllBytes (file, content); + + try + { + FileAccessPolicy policy = new (cwd, allowedFiles: null, allowBinary: true); + string? error = policy.CheckFile (file); + + Assert.Null (error); + } + finally + { + File.Delete (file); + } + } + + [Fact] + public void GlobAggregate_TooManyFiles_IsRefused () + { + // Create a list of file paths exceeding MaxGlobFiles + List files = []; + + for (int i = 0; i <= FileAccessPolicy.MaxGlobFiles; i++) + { + files.Add ($"/tmp/file{i}.md"); + } + + FileAccessPolicy policy = new (Path.GetTempPath (), allowedFiles: null, allowBinary: false); + string? error = policy.CheckGlobAggregate (files); + + Assert.NotNull (error); + Assert.Contains ("exceeding the maximum", error); + } + + [Fact] + public void OversizedFile_IsRefused () + { + string cwd = Path.GetTempPath (); + string file = Path.Combine (cwd, "large.md"); + + // Create a file slightly larger than 16 MiB + using (FileStream fs = File.Create (file)) + { + fs.SetLength (FileAccessPolicy.MaxFileSizeBytes + 1); + } + + try + { + FileAccessPolicy policy = new (cwd, allowedFiles: null, allowBinary: false); + string? error = policy.CheckFile (file); + + Assert.NotNull (error); + Assert.Contains ("per-file limit", error); + } + finally + { + File.Delete (file); + } + } + + [Fact] + public void TxtExtension_IsAllowed () + { + string cwd = Path.GetTempPath (); + string file = Path.Combine (cwd, "notes.txt"); + File.WriteAllText (file, "some notes"); + + try + { + FileAccessPolicy policy = new (cwd, allowedFiles: null, allowBinary: false); + string? error = policy.CheckFile (file); + + Assert.Null (error); + } + finally + { + File.Delete (file); + } + } + + [Fact] + public void MarkdownExtension_IsAllowed () + { + string cwd = Path.GetTempPath (); + string file = Path.Combine (cwd, "readme.markdown"); + File.WriteAllText (file, "# hello"); + + try + { + FileAccessPolicy policy = new (cwd, allowedFiles: null, allowBinary: false); + string? error = policy.CheckFile (file); + + Assert.Null (error); + } + finally + { + File.Delete (file); + } + } +}