Wrap all IFileSystem usage in ScopedFileSystem#3001
Open
Conversation
Introduces `Nullean.ScopedFileSystem` (v0.2.0) as a security boundary
around every file system operation in the codebase.
`ScopedFileSystem` is a `System.IO.Abstractions.IFileSystem` decorator
that enforces at runtime that no file read or write can escape a set of
configured root directories. Any path outside the roots — including paths
reached via symlink, `..` traversal, or hidden directories — throws a
`ScopedFileSystemException` (extends `SecurityException`) before the
underlying OS call is ever made. `File.Exists` / `Directory.Exists` return
`false` for out-of-scope paths instead of throwing, so probing code stays
safe. The library supports multiple disjoint roots, an allowlist for
specific hidden names (e.g. `.git`), and opt-in OS special-folder access
(e.g. `Temp`).
Changes:
- New `FileSystemFactory` in `Elastic.Documentation.Configuration` with:
- `FileSystemFactory.Real` — pre-allocated singleton scoped to the
working-directory root and `Paths.ApplicationData`
(`LocalApplicationData/elastic/docs-builder`), with `.git`
folder/file allowlisted and `Temp` special-folder enabled
- `FileSystemFactory.InMemory()` — wraps a fresh `MockFileSystem` in
the same scope options; each call returns a new independent instance
- `FileSystemFactory.CreateScoped(IFileSystem inner, ...)` — for
wrapping an existing FS with optional extra roots from extensions
- `FileSystemFactory.CreateForUserData()` — user-profile scope with
`.docs-builder` allowlisted, used by `GitLinkIndexReader`
- All 33+ `new FileSystem()` call sites replaced with factory methods
- `CrossLinkFetcher` and `CheckForUpdatesFilter` converted from direct
`System.IO.File` / `Directory` static calls to `IFileSystem` injection
- `IDocsBuilderExtension` gains `ExternalScopeRoots` (default `[]`);
`DetectionRulesDocsBuilderExtension` implements it to expose the
detection-rules folders so builds can widen the scope when needed
- `IFileSystem` registered as DI singleton (`FileSystemFactory.Real`)
in `DocumentationTooling`
- Service constructors that previously required the concrete `FileSystem`
type widened to `IFileSystem`
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces a centralized FileSystemFactory and migrates the codebase to use Nullean.ScopedFileSystem instances (scoped read/write/app-data/in-memory) instead of directly instantiating System.IO.Abstractions FileSystem or using raw IFileSystem in many places. Constructors, method parameters, and test setups were updated to accept ScopedFileSystem (or IFileSystem wrapped by the factory). Defaults that previously used new FileSystem() now use factory-provided instances. It also adds the Nullean.ScopedFileSystem package and exposes an ExternalScopeRoots property on IDocsBuilderExtension. Sequence Diagram(s)sequenceDiagram
participant CLI as CLI Command
participant Factory as FileSystemFactory
participant Scoped as ScopedFileSystem
participant Under as Underlying I/O FS
participant Service as Domain Service
participant ES as ElasticsearchEndpointConfigurator
participant Build as BuildService
CLI->>Factory: request filesystem (RealRead/AppData/InMemory)
Factory->>Scoped: create/return ScopedFileSystem
Scoped->>Under: delegate IO operations
CLI->>Service: invoke operation (passes ScopedFileSystem)
Service->>ES: ApplyAsync(endpoint, ScopedFileSystem)
Service->>Build: BuildAll(context, ScopedFileSystem)
Build->>Scoped: read/write repo files
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/tooling/docs-builder/Commands/ChangelogCommand.cs (1)
291-292:⚠️ Potential issue | 🟠 MajorBypasses scoped filesystem security boundary.
ChangelogConfigurationLoaderis instantiated withnew System.IO.Abstractions.FileSystem()instead of using_fileSystem(which isFileSystemFactory.Real). This same pattern repeats at lines 545, 848, and 1107.This defeats the PR's goal of wrapping all filesystem access in
ScopedFileSystem.🔧 Proposed fix
- var bundleConfig = await new ChangelogConfigurationLoader(logFactory, configurationContext, new System.IO.Abstractions.FileSystem()) + var bundleConfig = await new ChangelogConfigurationLoader(logFactory, configurationContext, _fileSystem) .LoadChangelogConfiguration(collector, config, ctx);Apply similar changes at lines 545, 848, and 1107.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tooling/docs-builder/Commands/ChangelogCommand.cs` around lines 291 - 292, The ChangelogConfigurationLoader is being constructed with a direct new System.IO.Abstractions.FileSystem() which bypasses the scoped filesystem; replace that explicit instantiation with the existing injected/created _fileSystem (FileSystemFactory.Real) when calling new ChangelogConfigurationLoader(...). Update the same pattern wherever ChangelogConfigurationLoader is constructed in this file (the other occurrences that currently use new System.IO.Abstractions.FileSystem()) so all filesystem access uses the ScopedFileSystem (_fileSystem) and not a fresh FileSystem instance; ensure the call to LoadChangelogConfiguration(collector, config, ctx) remains unchanged.src/tooling/docs-builder/Http/StaticWebHost.cs (1)
97-123:⚠️ Potential issue | 🔴 CriticalScoped filesystem boundary is bypassed during request-time file reads
ServeRootIndex/ServeDocumentationFileuseSystem.IO(FileInfo,DirectoryInfo,Path) directly, so scoped checks are skipped at runtime. This can allow serving content through in-root symlinks that resolve outside the allowed scope.Proposed fix
public class StaticWebHost { public WebApplication WebApplication { get; } + private readonly IFileSystem _fileSystem = FileSystemFactory.Real; private readonly string _contentRoot; public StaticWebHost(int port, string? path) { _contentRoot = path ?? Path.Join(Paths.WorkingDirectoryRoot.FullName, ".artifacts", "assembly"); - var fs = FileSystemFactory.Real; - var dir = fs.DirectoryInfo.New(_contentRoot); + var dir = _fileSystem.DirectoryInfo.New(_contentRoot); if (!dir.Exists) throw new Exception($"Can not serve empty directory: {_contentRoot}"); - if (!dir.IsSubPathOf(fs.DirectoryInfo.New(Paths.WorkingDirectoryRoot.FullName))) + if (!dir.IsSubPathOf(_fileSystem.DirectoryInfo.New(Paths.WorkingDirectoryRoot.FullName))) throw new Exception($"Can not serve directory outside of: {Paths.WorkingDirectoryRoot.FullName}"); @@ private Task<IResult> ServeRootIndex(Cancel _) { - var indexPath = Path.Join(_contentRoot, "index.html"); - var fileInfo = new FileInfo(indexPath); + var indexPath = _fileSystem.Path.Join(_contentRoot, "index.html"); + var fileInfo = _fileSystem.FileInfo.New(indexPath); if (fileInfo.Exists) return Task.FromResult(Results.File(fileInfo.FullName, "text/html")); @@ private async Task<IResult> ServeDocumentationFile(string slug, Cancel _) { @@ - var contentRoot = Path.GetFullPath(_contentRoot); - var localPath = Path.GetFullPath(Path.Join(contentRoot, slug.Replace('/', Path.DirectorySeparatorChar))); + var contentRoot = _fileSystem.Path.GetFullPath(_contentRoot); + var localPath = _fileSystem.Path.GetFullPath(Path.Join(contentRoot, slug.Replace('/', Path.DirectorySeparatorChar))); if (!localPath.StartsWith(contentRoot + Path.DirectorySeparatorChar, StringComparison.Ordinal)) return Results.NotFound(); - var fileInfo = new FileInfo(localPath); - var directoryInfo = new DirectoryInfo(localPath); + var fileInfo = _fileSystem.FileInfo.New(localPath); + var directoryInfo = _fileSystem.DirectoryInfo.New(localPath); if (directoryInfo.Exists) - fileInfo = new FileInfo(Path.Join(directoryInfo.FullName, "index.html")); + fileInfo = _fileSystem.FileInfo.New(Path.Join(directoryInfo.FullName, "index.html"));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tooling/docs-builder/Http/StaticWebHost.cs` around lines 97 - 123, ServeRootIndex and ServeDocumentationFile currently validate paths with Path.GetFullPath/StartsWith but a malicious symlink inside the content root can still point outside; fix by resolving any symbolic links for both files and directories before the sandbox check: use FileInfo.ResolveLinkTarget (and DirectoryInfo.ResolveLinkTarget for directories) in a loop to follow links to the final target, then compute the finalTargetFullPath and assert it starts with the sanitized contentRoot + Path.DirectorySeparatorChar; if it does not, return Results.NotFound(); apply this resolution/validation to the indexPath handling in ServeRootIndex and to localPath/fileInfo/directoryInfo handling in ServeDocumentationFile (use _contentRoot, contentRoot, localPath, fileInfo, directoryInfo identifiers from the diff).src/tooling/docs-builder/Filters/CheckForUpdatesFilter.cs (1)
65-83:⚠️ Potential issue | 🟠 MajorBest-effort update check can currently fail the command
ReadAllTextAsync(Line 67) runs outside a catch, and the later write path is inside atry/finally(no catch), so filesystem/security/network exceptions can bubble and fail execution after the main command already ran. This should degrade gracefully.Proposed fix
private async ValueTask<Uri?> GetLatestVersion(Cancel ctx) { // only check for new versions once per hour - if (_stateFile.Exists && _stateFile.LastWriteTimeUtc >= DateTime.UtcNow.Subtract(TimeSpan.FromHours(1))) - { - var url = await fileSystem.File.ReadAllTextAsync(_stateFile.FullName, ctx); - if (Uri.TryCreate(url, UriKind.Absolute, out var uri)) - return uri; - } + try + { + if (_stateFile.Exists && _stateFile.LastWriteTimeUtc >= DateTime.UtcNow.Subtract(TimeSpan.FromHours(1))) + { + var url = await fileSystem.File.ReadAllTextAsync(_stateFile.FullName, ctx); + if (Uri.TryCreate(url, UriKind.Absolute, out var uri)) + return uri; + } + } + catch + { + // ignore cache read failures; continue with network check + } try { var httpClient = new HttpClient(new HttpClientHandler { AllowAutoRedirect = false }); var response = await httpClient.GetAsync("https://github.com/elastic/docs-builder/releases/latest", ctx); var redirectUrl = response.Headers.Location; if (redirectUrl is not null && _stateFile.Directory is not null) { // ensure the 'elastic' folder exists. if (!fileSystem.Directory.Exists(_stateFile.Directory.FullName)) _ = fileSystem.Directory.CreateDirectory(_stateFile.Directory.FullName); await fileSystem.File.WriteAllTextAsync(_stateFile.FullName, redirectUrl.ToString(), ctx); } return redirectUrl; } - // ReSharper disable once RedundantEmptyFinallyBlock - // ignore on purpose - finally { } + catch + { + // ignore update check failures + return null; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tooling/docs-builder/Filters/CheckForUpdatesFilter.cs` around lines 65 - 83, The ReadAllTextAsync call on _stateFile and the HTTP/file write logic (HttpClient.GetAsync and fileSystem.File.WriteAllTextAsync) can throw and currently bubble up; change CheckForUpdatesFilter so both the read and the whole update-check block catch and swallow non-fatal exceptions (IO, security, network) and log them instead of rethrowing so the command degrades gracefully; specifically wrap the ReadAllTextAsync(_stateFile.FullName, ctx) in a try/catch that returns null on failure, and wrap the HTTP request + WriteAllTextAsync sequence in a try/catch that logs the exception and continues without failing the operation.src/services/Elastic.Documentation.Assembler/Deploying/IncrementalDeployService.cs (1)
53-55:⚠️ Potential issue | 🔴 CriticalUse
fileSystemfor file I/O to enforce scope checks.Line 53–55 use
new FileStream()andnew StreamWriter()directly instead of the injectedIFileSystemabstraction. Line 76 similarly usesFile.ReadAllTextAsync()instead offileSystem.File.ReadAllTextAsync(). This inconsistency bypasses scope enforcement that theScopedFileSystemabstraction provides elsewhere in the method (e.g., line 71).Replace all three locations with
fileSystem.File.WriteAllTextAsync()andfileSystem.File.ReadAllTextAsync()to maintain consistent scope handling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/Elastic.Documentation.Assembler/Deploying/IncrementalDeployService.cs` around lines 53 - 55, The code directly constructs FileStream and StreamWriter and calls File.ReadAllTextAsync() which bypasses the injected IFileSystem scope enforcement; replace the direct FileStream/StreamWriter usage and the File.ReadAllTextAsync() call with the fileSystem abstraction by using fileSystem.File.WriteAllTextAsync(outputPath, output) (or equivalent fileSystem.File.WriteAllTextAsync for the write) and fileSystem.File.ReadAllTextAsync(...) for the read so that IncrementalDeployService uses the injected fileSystem consistently (locate the write sites around the FileStream/StreamWriter creation and the read site that calls File.ReadAllTextAsync()).src/services/Elastic.Documentation.Assembler/Building/AssemblerBuildService.cs (1)
125-126:⚠️ Potential issue | 🟠 MajorUse scoped
fs.File.Exists()instead of staticFile.Exists().Line 125 uses the static
File.Exists()while the method accepts anIFileSystem fsparameter that's already used consistently elsewhere (line 73). This breaks the file system abstraction pattern, making the code harder to test and the behavior inconsistent with the scoped semantics you've established.Proposed fix
- if (File.Exists(redirectsPath)) + if (fs.File.Exists(redirectsPath)) await githubActionsService.SetOutputAsync("redirects-artifact-path", redirectsPath);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/Elastic.Documentation.Assembler/Building/AssemblerBuildService.cs` around lines 125 - 126, The code calls the static File.Exists when it should use the injected IFileSystem to respect the file-system abstraction; update the check in the method that uses the IFileSystem parameter (fs) to call fs.File.Exists(redirectsPath) before awaiting githubActionsService.SetOutputAsync("redirects-artifact-path", redirectsPath) so the behavior is testable and consistent with the other fs usages in AssemblerBuildService.
🧹 Nitpick comments (1)
src/services/Elastic.Documentation.Assembler/Deploying/DeployUpdateRedirectsService.cs (1)
15-15: Use the injectedIFileSystemconsistently.Line 45 uses
Directory.GetCurrentDirectory()directly instead offileSystem.Directory.GetCurrentDirectory(). The constructor acceptsIFileSystemfor dependency injection, but this direct static call bypasses the abstraction, breaking testability and mockability. The codebase consistently uses thefileSystem.Directorypattern elsewhere (see ChangelogConfigurationLoader, ChangelogFileWriter, etc.).Replace:
fileSystem.DirectoryInfo.New(Directory.GetCurrentDirectory())With:
fileSystem.DirectoryInfo.New(fileSystem.Directory.GetCurrentDirectory())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/Elastic.Documentation.Assembler/Deploying/DeployUpdateRedirectsService.cs` at line 15, In DeployUpdateRedirectsService update the call that creates a DirectoryInfo so it uses the injected IFileSystem instead of the static Directory API: replace the use of Directory.GetCurrentDirectory() when constructing fileSystem.DirectoryInfo.New(...) so it reads fileSystem.Directory.GetCurrentDirectory(); locate this in the DeployUpdateRedirectsService constructor/initialization where fileSystem.DirectoryInfo.New(...) is invoked and make the single-argument change to use fileSystem.Directory.GetCurrentDirectory() to preserve testability and consistent IFileSystem usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Elastic.Documentation.LinkIndex/GitLinkIndexReader.cs`:
- Around line 36-43: The constructor currently allows an injected IFileSystem to
bypass scoping; always wrap the provided or a new FileSystem inside a
ScopedFileSystem to enforce the runtime security boundary. Replace the
null-coalescing that assigns _fileSystem with code that constructs new
ScopedFileSystem(inner: fileSystem ?? new FileSystem(), options: new
ScopedFileSystemOptions(...)) so the injected instance becomes the inner layer;
keep the existing AllowedHiddenFolderNames and AllowedHiddenFileNames in the
ScopedFileSystemOptions and assign the resulting ScopedFileSystem to _fileSystem
in the GitLinkIndexReader constructor.
In `@src/Elastic.Markdown/Myst/Directives/CsvInclude/CsvReader.cs`:
- Around line 13-16: ReadWithSep currently special-cases a concrete
MockFileSystem and falls back to Sep.FromFile(filePath), which bypasses the
injected IFileSystem (FileSystemFactory.Real / InMemory) and breaks scoped
mocks; update ReadWithSep to accept an IFileSystem parameter (or add an
overload) and use that IFileSystem to read the file/derive the separator instead
of calling Sep.FromFile(filePath), ensuring ReadCsvFile (which already passes
fs) and any callers use the injected IFileSystem; reference ReadCsvFile,
ReadWithSep, IFileSystem, FileSystemFactory.Real / FileSystemFactory.InMemory
and Sep.FromFile when making the change.
---
Outside diff comments:
In
`@src/services/Elastic.Documentation.Assembler/Building/AssemblerBuildService.cs`:
- Around line 125-126: The code calls the static File.Exists when it should use
the injected IFileSystem to respect the file-system abstraction; update the
check in the method that uses the IFileSystem parameter (fs) to call
fs.File.Exists(redirectsPath) before awaiting
githubActionsService.SetOutputAsync("redirects-artifact-path", redirectsPath) so
the behavior is testable and consistent with the other fs usages in
AssemblerBuildService.
In
`@src/services/Elastic.Documentation.Assembler/Deploying/IncrementalDeployService.cs`:
- Around line 53-55: The code directly constructs FileStream and StreamWriter
and calls File.ReadAllTextAsync() which bypasses the injected IFileSystem scope
enforcement; replace the direct FileStream/StreamWriter usage and the
File.ReadAllTextAsync() call with the fileSystem abstraction by using
fileSystem.File.WriteAllTextAsync(outputPath, output) (or equivalent
fileSystem.File.WriteAllTextAsync for the write) and
fileSystem.File.ReadAllTextAsync(...) for the read so that
IncrementalDeployService uses the injected fileSystem consistently (locate the
write sites around the FileStream/StreamWriter creation and the read site that
calls File.ReadAllTextAsync()).
In `@src/tooling/docs-builder/Commands/ChangelogCommand.cs`:
- Around line 291-292: The ChangelogConfigurationLoader is being constructed
with a direct new System.IO.Abstractions.FileSystem() which bypasses the scoped
filesystem; replace that explicit instantiation with the existing
injected/created _fileSystem (FileSystemFactory.Real) when calling new
ChangelogConfigurationLoader(...). Update the same pattern wherever
ChangelogConfigurationLoader is constructed in this file (the other occurrences
that currently use new System.IO.Abstractions.FileSystem()) so all filesystem
access uses the ScopedFileSystem (_fileSystem) and not a fresh FileSystem
instance; ensure the call to LoadChangelogConfiguration(collector, config, ctx)
remains unchanged.
In `@src/tooling/docs-builder/Filters/CheckForUpdatesFilter.cs`:
- Around line 65-83: The ReadAllTextAsync call on _stateFile and the HTTP/file
write logic (HttpClient.GetAsync and fileSystem.File.WriteAllTextAsync) can
throw and currently bubble up; change CheckForUpdatesFilter so both the read and
the whole update-check block catch and swallow non-fatal exceptions (IO,
security, network) and log them instead of rethrowing so the command degrades
gracefully; specifically wrap the ReadAllTextAsync(_stateFile.FullName, ctx) in
a try/catch that returns null on failure, and wrap the HTTP request +
WriteAllTextAsync sequence in a try/catch that logs the exception and continues
without failing the operation.
In `@src/tooling/docs-builder/Http/StaticWebHost.cs`:
- Around line 97-123: ServeRootIndex and ServeDocumentationFile currently
validate paths with Path.GetFullPath/StartsWith but a malicious symlink inside
the content root can still point outside; fix by resolving any symbolic links
for both files and directories before the sandbox check: use
FileInfo.ResolveLinkTarget (and DirectoryInfo.ResolveLinkTarget for directories)
in a loop to follow links to the final target, then compute the
finalTargetFullPath and assert it starts with the sanitized contentRoot +
Path.DirectorySeparatorChar; if it does not, return Results.NotFound(); apply
this resolution/validation to the indexPath handling in ServeRootIndex and to
localPath/fileInfo/directoryInfo handling in ServeDocumentationFile (use
_contentRoot, contentRoot, localPath, fileInfo, directoryInfo identifiers from
the diff).
---
Nitpick comments:
In
`@src/services/Elastic.Documentation.Assembler/Deploying/DeployUpdateRedirectsService.cs`:
- Line 15: In DeployUpdateRedirectsService update the call that creates a
DirectoryInfo so it uses the injected IFileSystem instead of the static
Directory API: replace the use of Directory.GetCurrentDirectory() when
constructing fileSystem.DirectoryInfo.New(...) so it reads
fileSystem.Directory.GetCurrentDirectory(); locate this in the
DeployUpdateRedirectsService constructor/initialization where
fileSystem.DirectoryInfo.New(...) is invoked and make the single-argument change
to use fileSystem.Directory.GetCurrentDirectory() to preserve testability and
consistent IFileSystem usage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 69eead30-c3dd-41dc-9d2a-8e9fc6b20bf1
📒 Files selected for processing (53)
Directory.Packages.propssrc/Elastic.Codex/Indexing/CodexIndexService.cssrc/Elastic.Documentation.Configuration/ConfigurationFileProvider.cssrc/Elastic.Documentation.Configuration/Elastic.Documentation.Configuration.csprojsrc/Elastic.Documentation.Configuration/FileSystemFactory.cssrc/Elastic.Documentation.Configuration/Paths.cssrc/Elastic.Documentation.LinkIndex/Elastic.Documentation.LinkIndex.csprojsrc/Elastic.Documentation.LinkIndex/GitLinkIndexReader.cssrc/Elastic.Documentation.Links/CrossLinks/CrossLinkFetcher.cssrc/Elastic.Markdown/Extensions/DetectionRules/DetectionRulesDocsBuilderExtension.cssrc/Elastic.Markdown/Extensions/IDocsBuilderExtension.cssrc/Elastic.Markdown/Myst/Directives/CsvInclude/CsvReader.cssrc/authoring/Elastic.Documentation.Refactor/Tracking/LocalChangesService.cssrc/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cssrc/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cssrc/services/Elastic.Changelog/Bundling/ChangelogRemoveService.cssrc/services/Elastic.Changelog/Bundling/PromotionReportParser.cssrc/services/Elastic.Changelog/Creation/ChangelogCreationService.cssrc/services/Elastic.Changelog/Evaluation/ChangelogPrEvaluationService.cssrc/services/Elastic.Changelog/GithubRelease/GitHubReleaseChangelogService.cssrc/services/Elastic.Changelog/Rendering/ChangelogRenderingService.cssrc/services/Elastic.Documentation.Assembler/Building/AssemblerBuildService.cssrc/services/Elastic.Documentation.Assembler/Building/AssemblerSitemapService.cssrc/services/Elastic.Documentation.Assembler/Configuration/ConfigurationCloneService.cssrc/services/Elastic.Documentation.Assembler/ContentSources/RepositoryBuildMatchingService.cssrc/services/Elastic.Documentation.Assembler/ContentSources/RepositoryPublishValidationService.cssrc/services/Elastic.Documentation.Assembler/Deploying/DeployUpdateRedirectsService.cssrc/services/Elastic.Documentation.Assembler/Deploying/IncrementalDeployService.cssrc/services/Elastic.Documentation.Assembler/Indexing/AssemblerIndexService.cssrc/services/Elastic.Documentation.Assembler/Sourcing/AssemblerCloneService.cssrc/services/Elastic.Documentation.Isolated/IsolatedIndexService.cssrc/tooling/docs-builder/Commands/Assembler/AssemblerCommands.cssrc/tooling/docs-builder/Commands/Assembler/AssemblerIndexCommand.cssrc/tooling/docs-builder/Commands/Assembler/AssemblerSitemapCommand.cssrc/tooling/docs-builder/Commands/Assembler/ConfigurationCommands.cssrc/tooling/docs-builder/Commands/Assembler/ContentSourceCommands.cssrc/tooling/docs-builder/Commands/Assembler/DeployCommands.cssrc/tooling/docs-builder/Commands/Assembler/NavigationCommands.cssrc/tooling/docs-builder/Commands/ChangelogCommand.cssrc/tooling/docs-builder/Commands/Codex/CodexCommands.cssrc/tooling/docs-builder/Commands/Codex/CodexIndexCommand.cssrc/tooling/docs-builder/Commands/Codex/CodexUpdateRedirectsCommand.cssrc/tooling/docs-builder/Commands/DiffCommands.cssrc/tooling/docs-builder/Commands/FormatCommand.cssrc/tooling/docs-builder/Commands/InboundLinkCommands.cssrc/tooling/docs-builder/Commands/IndexCommand.cssrc/tooling/docs-builder/Commands/IsolatedBuildCommand.cssrc/tooling/docs-builder/Commands/MoveCommand.cssrc/tooling/docs-builder/Commands/ServeCommand.cssrc/tooling/docs-builder/DocumentationTooling.cssrc/tooling/docs-builder/Filters/CheckForUpdatesFilter.cssrc/tooling/docs-builder/Http/InMemoryBuildState.cssrc/tooling/docs-builder/Http/StaticWebHost.cs
Fix a critical bug where real disk builds would throw ScopedFileSystemException
when writing to the default output directory (.artifacts/docs/html), because
.artifacts was missing from AllowedHiddenFolderNames.
Changes:
- FileSystemFactory.Real: remove speculative hidden-file allowances
(.gitignore, .gitmodules, .gitattributes, .editorconfig, .nojekyll — none
accessed via IFileSystem), remove AllowedSpecialFolders.Temp, add
.artifacts (folder) and .doc.state (file) which are confirmed accessed
- FileSystemFactory.AppData: new pre-allocated FS scoped to only the elastic
/docs-builder app data directory; used by components that never touch
workspace files (CrossLinkFetcher, CheckForUpdatesFilter, GitLinkIndexReader)
- ConfigurationFileProvider: move temp staging directory from OS temp to
{ApplicationData}/config-runtime/, eliminating the only Temp usage
- GitLinkIndexReader: normalize CloneDirectory from ~/.docs-builder/codex-link-index
to {LocalAppData}/elastic/docs-builder/codex-link-index so all app data
lives under one root; switch FS fallback from custom user-profile scope
to FileSystemFactory.AppData
- CrossLinkFetcher: default FS changed to FileSystemFactory.AppData
- CheckForUpdatesFilter: use FileSystemFactory.AppData directly; remove
IFileSystem DI injection (AppData is the correct and only scope needed)
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Builds that write HTML output never need .git access. Introducing RealWrite enforces this at the ScopedFileSystem level: any accidental write into .git would throw ScopedFileSystemException rather than silently succeeding. RealRead retains .git access (needed by GitCheckoutInformation for branch/remote metadata and worktree resolution), plus .artifacts and .doc.state for incremental build state reads. RealWrite has the same scope roots but omits .git from both AllowedHiddenFolderNames and AllowedHiddenFileNames. IsolatedBuildCommand now passes FileSystemFactory.RealWrite explicitly as the writeFileSystem argument for non-in-memory builds. In-memory builds pass null so IsolatedBuildService falls back to the same MockFileSystem used for reads. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/Elastic.Documentation.LinkIndex/GitLinkIndexReader.cs (1)
29-36:⚠️ Potential issue | 🟠 MajorEnforce scoping even when
fileSystemis injectedAt Line 35, the null-coalescing assignment allows a caller to pass an unscoped
IFileSystem, which bypasses the runtime filesystem security boundary this PR is introducing.Suggested fix
public GitLinkIndexReader(string environment, IFileSystem? fileSystem = null, bool skipFetch = false) { if (string.IsNullOrWhiteSpace(environment)) throw new ArgumentException("Environment must be specified in the codex configuration (e.g., 'internal', 'security').", nameof(environment)); _environment = environment; - _fileSystem = fileSystem ?? FileSystemFactory.AppData; + _fileSystem = fileSystem is null + ? FileSystemFactory.AppData + : FileSystemFactory.CreateScoped(fileSystem); _skipFetch = skipFetch; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Elastic.Documentation.LinkIndex/GitLinkIndexReader.cs` around lines 29 - 36, The constructor for GitLinkIndexReader currently assigns _fileSystem = fileSystem ?? FileSystemFactory.AppData which lets an unscoped IFileSystem bypass the new runtime security boundary; instead, ensure any injected fileSystem is converted/wrapped into a scoped instance before assignment (e.g., use the FileSystemFactory’s scoping API to produce a scoped IFileSystem for the given environment) so that _fileSystem always enforces the environment scope even when a non-null IFileSystem is passed into GitLinkIndexReader.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/Elastic.Documentation.LinkIndex/GitLinkIndexReader.cs`:
- Around line 29-36: The constructor for GitLinkIndexReader currently assigns
_fileSystem = fileSystem ?? FileSystemFactory.AppData which lets an unscoped
IFileSystem bypass the new runtime security boundary; instead, ensure any
injected fileSystem is converted/wrapped into a scoped instance before
assignment (e.g., use the FileSystemFactory’s scoping API to produce a scoped
IFileSystem for the given environment) so that _fileSystem always enforces the
environment scope even when a non-null IFileSystem is passed into
GitLinkIndexReader.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 759c70d1-0db2-47ba-9cb2-e987da938346
📒 Files selected for processing (6)
src/Elastic.Documentation.Configuration/ConfigurationFileProvider.cssrc/Elastic.Documentation.Configuration/FileSystemFactory.cssrc/Elastic.Documentation.LinkIndex/Elastic.Documentation.LinkIndex.csprojsrc/Elastic.Documentation.LinkIndex/GitLinkIndexReader.cssrc/Elastic.Documentation.Links/CrossLinks/CrossLinkFetcher.cssrc/tooling/docs-builder/Filters/CheckForUpdatesFilter.cs
✅ Files skipped from review due to trivial changes (1)
- src/Elastic.Documentation.LinkIndex/Elastic.Documentation.LinkIndex.csproj
🚧 Files skipped from review as they are similar to previous changes (3)
- src/Elastic.Documentation.Configuration/ConfigurationFileProvider.cs
- src/Elastic.Documentation.Links/CrossLinks/CrossLinkFetcher.cs
- src/Elastic.Documentation.Configuration/FileSystemFactory.cs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
RFC-nav-v2-migration-overview.md (1)
211-213: Add language specifiers to CLI examples.Same issue as the main RFC — lines 211, 231, and 243 have fenced code blocks without language specifiers.
📝 Proposed fix
-``` +```shell docs-builder assembler apply-nav-restructure --repo docs-contentApply similarly to lines 231 and 243. </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@RFC-nav-v2-migration-overview.mdaround lines 211 - 213, The fenced code
blocks containing the CLI command docs-builder assembler apply-nav-restructure
--repo docs-content (and the two other similar blocks at the same spots) lack
language specifiers; update each triple-backtick fence that wraps that command
to include a shell language tag (e.g., ```shell) so the blocks at the
occurrences of "docs-builder assembler apply-nav-restructure --repo
docs-content" render correctly and get shell syntax highlighting.</details> </blockquote></details> <details> <summary>RFC-nav-v2-migration.md (1)</summary><blockquote> `392-395`: **Add language specifier to fenced code blocks.** The CLI command examples at lines 392, 415, and 435 are missing language specifiers, which triggers markdownlint warnings. <details> <summary>📝 Proposed fix</summary> ```diff -``` +```shell docs-builder assembler validate-nav-migration ``` ``` Apply the same fix to lines 435 (`docs-builder assembler apply-nav-restructure`) and the example output blocks at lines 415 and 243. </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@RFC-nav-v2-migration.md` around lines 392 - 395, Add a language specifier (e.g., "shell") to the fenced code blocks that contain the CLI commands and example outputs so markdownlint warnings are resolved; update the blocks containing the commands "docs-builder assembler validate-nav-migration" and "docs-builder assembler apply-nav-restructure" and the example output blocks that show CLI results (the blocks containing the example output text) by changing their opening triple-backticks to include "shell" (or an appropriate language) so the fenced blocks are correctly annotated. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@src/tooling/docs-builder/Commands/ChangelogCommand.cs:
- Line 42: Replace direct instantiations of System.IO.Abstractions.FileSystem
passed into ChangelogConfigurationLoader with the scoped filesystem instance:
use the existing _fileSystem field (or FileSystemFactory.RealRead where
appropriate) instead of new System.IO.Abstractions.FileSystem() in the
ChangelogCommand class; update the calls that create a
ChangelogConfigurationLoader in the Create, Bundle, Remove and GitHubRelease
command handlers so they pass _fileSystem (or FileSystemFactory.RealRead) to
ChangelogConfigurationLoader's constructor to ensure all filesystem operations
are scoped consistently.
Nitpick comments:
In@RFC-nav-v2-migration-overview.md:
- Around line 211-213: The fenced code blocks containing the CLI command
docs-builder assembler apply-nav-restructure --repo docs-content (and the two
other similar blocks at the same spots) lack language specifiers; update each
triple-backtick fence that wraps that command to include a shell language tag
(e.g., ```shell) so the blocks at the occurrences of "docs-builder assembler
apply-nav-restructure --repo docs-content" render correctly and get shell syntax
highlighting.In
@RFC-nav-v2-migration.md:
- Around line 392-395: Add a language specifier (e.g., "shell") to the fenced
code blocks that contain the CLI commands and example outputs so markdownlint
warnings are resolved; update the blocks containing the commands "docs-builder
assembler validate-nav-migration" and "docs-builder assembler
apply-nav-restructure" and the example output blocks that show CLI results (the
blocks containing the example output text) by changing their opening
triple-backticks to include "shell" (or an appropriate language) so the fenced
blocks are correctly annotated.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Pro **Run ID**: `97c9a6f7-08bc-486d-bdf2-ee3beb598fee` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 4aa7056b0d34e5c0821b1fff34af2dc4be7b115a and fc45b448e834035363245162b3897030a2cf57a8. </details> <details> <summary>⛔ Files ignored due to path filters (14)</summary> * `rfc-diagrams/1-v1-architecture-path-flow.png` is excluded by `!**/*.png` * `rfc-diagrams/1-v1-architecture-path-flow.svg` is excluded by `!**/*.svg` * `rfc-diagrams/2-per-repo-loading-flow.png` is excluded by `!**/*.png` * `rfc-diagrams/2-per-repo-loading-flow.svg` is excluded by `!**/*.svg` * `rfc-diagrams/3-virtual-remap-example.png` is excluded by `!**/*.png` * `rfc-diagrams/3-virtual-remap-example.svg` is excluded by `!**/*.svg` * `rfc-diagrams/4-dual-path-providers.png` is excluded by `!**/*.png` * `rfc-diagrams/4-dual-path-providers.svg` is excluded by `!**/*.svg` * `rfc-diagrams/5-per-repo-migration-lifecycle.png` is excluded by `!**/*.png` * `rfc-diagrams/5-per-repo-migration-lifecycle.svg` is excluded by `!**/*.svg` * `rfc-diagrams/6-dual-validation-build-flow.png` is excluded by `!**/*.png` * `rfc-diagrams/6-dual-validation-build-flow.svg` is excluded by `!**/*.svg` * `rfc-diagrams/7-integration-pipeline.png` is excluded by `!**/*.png` * `rfc-diagrams/7-integration-pipeline.svg` is excluded by `!**/*.svg` </details> <details> <summary>📒 Files selected for processing (34)</summary> * `RFC-nav-v2-migration-overview.md` * `RFC-nav-v2-migration.md` * `src/Elastic.Documentation.Configuration/ConfigurationFileProvider.cs` * `src/Elastic.Documentation.Configuration/FileSystemFactory.cs` * `src/Elastic.Markdown/Myst/Directives/CsvInclude/CsvReader.cs` * `src/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cs` * `src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs` * `src/services/Elastic.Changelog/Bundling/ChangelogRemoveService.cs` * `src/services/Elastic.Changelog/Bundling/PromotionReportParser.cs` * `src/services/Elastic.Changelog/Creation/ChangelogCreationService.cs` * `src/services/Elastic.Changelog/Evaluation/ChangelogPrEvaluationService.cs` * `src/services/Elastic.Changelog/GithubRelease/GitHubReleaseChangelogService.cs` * `src/services/Elastic.Changelog/Rendering/ChangelogRenderingService.cs` * `src/services/Elastic.Documentation.Assembler/Sourcing/AssemblerCloneService.cs` * `src/tooling/docs-builder/Commands/Assembler/AssemblerCommands.cs` * `src/tooling/docs-builder/Commands/Assembler/AssemblerIndexCommand.cs` * `src/tooling/docs-builder/Commands/Assembler/AssemblerSitemapCommand.cs` * `src/tooling/docs-builder/Commands/Assembler/ConfigurationCommands.cs` * `src/tooling/docs-builder/Commands/Assembler/ContentSourceCommands.cs` * `src/tooling/docs-builder/Commands/Assembler/DeployCommands.cs` * `src/tooling/docs-builder/Commands/Assembler/NavigationCommands.cs` * `src/tooling/docs-builder/Commands/ChangelogCommand.cs` * `src/tooling/docs-builder/Commands/Codex/CodexCommands.cs` * `src/tooling/docs-builder/Commands/Codex/CodexIndexCommand.cs` * `src/tooling/docs-builder/Commands/Codex/CodexUpdateRedirectsCommand.cs` * `src/tooling/docs-builder/Commands/DiffCommands.cs` * `src/tooling/docs-builder/Commands/FormatCommand.cs` * `src/tooling/docs-builder/Commands/InboundLinkCommands.cs` * `src/tooling/docs-builder/Commands/IndexCommand.cs` * `src/tooling/docs-builder/Commands/IsolatedBuildCommand.cs` * `src/tooling/docs-builder/Commands/MoveCommand.cs` * `src/tooling/docs-builder/Commands/ServeCommand.cs` * `src/tooling/docs-builder/Http/InMemoryBuildState.cs` * `src/tooling/docs-builder/Http/StaticWebHost.cs` </details> <details> <summary>✅ Files skipped from review due to trivial changes (8)</summary> * src/tooling/docs-builder/Commands/FormatCommand.cs * src/tooling/docs-builder/Commands/MoveCommand.cs * src/tooling/docs-builder/Commands/Assembler/AssemblerSitemapCommand.cs * src/tooling/docs-builder/Commands/Assembler/AssemblerCommands.cs * src/tooling/docs-builder/Commands/Assembler/AssemblerIndexCommand.cs * src/tooling/docs-builder/Commands/Codex/CodexCommands.cs * src/tooling/docs-builder/Commands/Assembler/DeployCommands.cs * src/tooling/docs-builder/Http/InMemoryBuildState.cs </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (13)</summary> * src/Elastic.Markdown/Myst/Directives/CsvInclude/CsvReader.cs * src/tooling/docs-builder/Commands/Assembler/ConfigurationCommands.cs * src/services/Elastic.Changelog/Bundling/ChangelogRemoveService.cs * src/tooling/docs-builder/Commands/Codex/CodexUpdateRedirectsCommand.cs * src/services/Elastic.Documentation.Assembler/Sourcing/AssemblerCloneService.cs * src/tooling/docs-builder/Commands/ServeCommand.cs * src/tooling/docs-builder/Commands/DiffCommands.cs * src/services/Elastic.Changelog/Evaluation/ChangelogPrEvaluationService.cs * src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs * src/services/Elastic.Changelog/GithubRelease/GitHubReleaseChangelogService.cs * src/tooling/docs-builder/Http/StaticWebHost.cs * src/tooling/docs-builder/Commands/Assembler/ContentSourceCommands.cs * src/tooling/docs-builder/Commands/InboundLinkCommands.cs </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Pass FileSystemFactory.AppData to GitLinkIndexReader at the three call
sites that previously passed context.ReadFileSystem or the build fileSystem
parameter. GitLinkIndexReader only ever accesses {AppData}/codex-link-index
so the workspace scope was unnecessarily broad. The IFileSystem? parameter
is retained for test injection.
Rename CreateScoped -> WrapToRead (read options, .git allowed) and add
WrapToWrite (write options, .git not allowed) to match the RealRead/RealWrite
naming convention. The extension-roots overload WrapToRead(inner, extensionRoots)
is the hook for DetectionRules external scope wiring.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Addresses the CodeRabbit concern: services accepted IFileSystem? with ??
defaults, allowing any unscoped instance to bypass the security boundary
silently. The fix is a compile-time contract — no runtime is/as checks.
Changes:
- FileSystemFactory: return types changed from IFileSystem to ScopedFileSystem
for all factory members (RealRead, RealWrite, AppData, InMemory,
WrapToRead, WrapToWrite). Logic unchanged; this is annotation only.
- IDocumentationContext: ReadFileSystem/WriteFileSystem typed ScopedFileSystem
- BuildContext: properties and constructor params typed ScopedFileSystem
- 12 optional IFileSystem? service parameters → ScopedFileSystem?
(GitLinkIndexReader, CrossLinkFetcher, CsvReader, all changelog services,
IsolatedBuildService.writeFileSystem)
- DocumentationSetFile.LoadAndResolve: ScopedFileSystem? with cast guard on
the IFileInfo/IDirectoryInfo.FileSystem fallback
- AssembleContext, CodexContext, authoring services: updated to ScopedFileSystem
- All IDocumentationContext mock implementations in tests updated
- Tests: new MockFileSystem() → FileSystemFactory.WrapToRead(new MockFileSystem())
new FileSystem() → FileSystemFactory.RealRead
The compiler now rejects any call site that tries to pass an unscoped
new FileSystem() or bare MockFileSystem to a service boundary.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/Elastic.Codex/CodexContext.cs (1)
52-53:⚠️ Potential issue | 🟠 MajorMaterialize
OutputDirectoryfromWriteFileSystem, notReadFileSystem.You now accept separate read/write filesystems, but
OutputDirectoryis still created fromReadFileSystemon Line 53. If scopes differ, writes can target a directory that wasn’t resolved in the write scope.Suggested fix
var defaultOutputDirectory = Path.Join(Paths.WorkingDirectoryRoot.FullName, ".artifacts", "codex", "docs"); - OutputDirectory = ReadFileSystem.DirectoryInfo.New(outputDirectory ?? defaultOutputDirectory); + OutputDirectory = WriteFileSystem.DirectoryInfo.New(outputDirectory ?? defaultOutputDirectory); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Elastic.Codex/CodexContext.cs` around lines 52 - 53, OutputDirectory is currently materialized using ReadFileSystem.DirectoryInfo.New; change it to use WriteFileSystem.DirectoryInfo.New so the directory is resolved in the write scope (replace ReadFileSystem.DirectoryInfo.New(...) with WriteFileSystem.DirectoryInfo.New(...), applied to the OutputDirectory assignment that uses defaultOutputDirectory and the optional outputDirectory parameter). Ensure you reference the same defaultOutputDirectory calculation and the OutputDirectory property so writes target the write filesystem.src/services/Elastic.Documentation.Assembler/Building/AssemblerBuildService.cs (1)
126-127:⚠️ Potential issue | 🟠 MajorUse scoped filesystem for existence checks.
Line 126 uses
System.IO.File.Exists, which bypasses the scoped filesystem boundary. Usefs.File.Exists(redirectsPath)instead—the same method already uses the scoped filesystem pattern elsewhere, and this ensures consistent boundary enforcement.Suggested fix
- if (File.Exists(redirectsPath)) + if (fs.File.Exists(redirectsPath)) await githubActionsService.SetOutputAsync("redirects-artifact-path", redirectsPath);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/Elastic.Documentation.Assembler/Building/AssemblerBuildService.cs` around lines 126 - 127, Replace the direct System.IO file existence check with the scoped filesystem's check: in AssemblerBuildService (method containing the redirectsPath logic) change the use of File.Exists(redirectsPath) to use the injected/scoped fs.File.Exists(redirectsPath) so the existence check respects the scoped filesystem boundary before calling githubActionsService.SetOutputAsync("redirects-artifact-path", redirectsPath).src/services/Elastic.Documentation.Assembler/AssembleContext.cs (1)
60-102:⚠️ Potential issue | 🟠 MajorUse
WriteFileSystemwhen initializingOutputDirectory.Line 95 initializes
OutputDirectoryfromReadFileSystem, but the property represents the output/write directory. Test cases inNavigationRootTests.cs:50andNavigationBuildingTests.cs:50pass different read and write filesystem instances, confirming the architecture supports scoped filesystems. Using the wrong instance here breaks scope enforcement.Fix
- OutputDirectory = ReadFileSystem.DirectoryInfo.New(output ?? defaultOutputDirectory); + OutputDirectory = WriteFileSystem.DirectoryInfo.New(output ?? defaultOutputDirectory);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/Elastic.Documentation.Assembler/AssembleContext.cs` around lines 60 - 102, AssembleContext's constructor incorrectly uses ReadFileSystem to create OutputDirectory; change the creation of OutputDirectory to use WriteFileSystem.DirectoryInfo.New(output ?? defaultOutputDirectory) so the output directory is created in the write-scoped filesystem. Keep CheckoutDirectory created from ReadFileSystem but ensure OutputDirectory and subsequent OutputWithPathPrefixDirectory are based on WriteFileSystem (refer to AssembleContext, OutputDirectory, OutputWithPathPrefixDirectory, ReadFileSystem, WriteFileSystem).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/services/Elastic.Changelog/Rendering/ChangelogRenderingService.cs`:
- Line 73: The field initialization uses FileSystemFactory.RealRead which
prevents writes and causes access denied errors when this service creates
directories or when ChangelogRenderer calls WriteAllTextAsync; change the
initialization of the ScopedFileSystem field (named _fileSystem) to use
FileSystemFactory.RealWrite instead of RealRead so the service can create
directories and write changelog files (keeps same scope as RealRead and avoids
writing .git metadata).
In
`@tests-integration/Elastic.Assembler.IntegrationTests/NavigationBuildingTests.cs`:
- Line 50: AssembleContext is being constructed with the second filesystem
wrapped using FileSystemFactory.WrapToRead, which grants read access to .git
metadata; change the second filesystem argument to use
FileSystemFactory.WrapToWrite(new MockFileSystem()) so the writeFileSystem
parameter gets the correct write-wrapped FS and WriteOptions block .git access
(locate the AssembleContext(...) call and replace
FileSystemFactory.WrapToRead(new MockFileSystem()) with
FileSystemFactory.WrapToWrite(new MockFileSystem())).
In `@tests/Navigation.Tests/Codex/CodexNavigationTestBase.cs`:
- Around line 83-84: The WriteFileSystem property is mistakenly wrapping
_fileSystem with FileSystemFactory.WrapToRead instead of using the write
wrapper; change the WriteFileSystem getter to return
FileSystemFactory.WrapToWrite(_fileSystem) (leave ReadFileSystem as WrapToRead).
Update CodexNavigationTestBase's WriteFileSystem property (and any other test
classes with the same pattern) to use WrapToWrite so write-mode semantics are
applied where tests call WriteFileSystem (e.g., mover.Execute()).
In `@tests/Navigation.Tests/Codex/GroupNavigationTests.cs`:
- Line 142: The WriteFileSystem property is incorrectly returning a read-scoped
wrapper; update the property so it returns a write-scoped wrapper by replacing
the use of FileSystemFactory.WrapToRead(_fs) with
FileSystemFactory.WrapToWrite(_fs), ensuring WriteFileSystem uses the write
scope (which enforces .git restrictions) rather than the read scope for _fs.
---
Outside diff comments:
In `@src/Elastic.Codex/CodexContext.cs`:
- Around line 52-53: OutputDirectory is currently materialized using
ReadFileSystem.DirectoryInfo.New; change it to use
WriteFileSystem.DirectoryInfo.New so the directory is resolved in the write
scope (replace ReadFileSystem.DirectoryInfo.New(...) with
WriteFileSystem.DirectoryInfo.New(...), applied to the OutputDirectory
assignment that uses defaultOutputDirectory and the optional outputDirectory
parameter). Ensure you reference the same defaultOutputDirectory calculation and
the OutputDirectory property so writes target the write filesystem.
In `@src/services/Elastic.Documentation.Assembler/AssembleContext.cs`:
- Around line 60-102: AssembleContext's constructor incorrectly uses
ReadFileSystem to create OutputDirectory; change the creation of OutputDirectory
to use WriteFileSystem.DirectoryInfo.New(output ?? defaultOutputDirectory) so
the output directory is created in the write-scoped filesystem. Keep
CheckoutDirectory created from ReadFileSystem but ensure OutputDirectory and
subsequent OutputWithPathPrefixDirectory are based on WriteFileSystem (refer to
AssembleContext, OutputDirectory, OutputWithPathPrefixDirectory, ReadFileSystem,
WriteFileSystem).
In
`@src/services/Elastic.Documentation.Assembler/Building/AssemblerBuildService.cs`:
- Around line 126-127: Replace the direct System.IO file existence check with
the scoped filesystem's check: in AssemblerBuildService (method containing the
redirectsPath logic) change the use of File.Exists(redirectsPath) to use the
injected/scoped fs.File.Exists(redirectsPath) so the existence check respects
the scoped filesystem boundary before calling
githubActionsService.SetOutputAsync("redirects-artifact-path", redirectsPath).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4dfe1b12-c857-4ddc-9ac5-8db10bfa1a73
📒 Files selected for processing (78)
src/Elastic.Codex/Building/CodexBuildService.cssrc/Elastic.Codex/CodexContext.cssrc/Elastic.Codex/Indexing/CodexIndexService.cssrc/Elastic.Documentation.Configuration/BuildContext.cssrc/Elastic.Documentation.Configuration/FileSystemFactory.cssrc/Elastic.Documentation.Configuration/Toc/DocumentationSetFile.cssrc/Elastic.Documentation.LinkIndex/GitLinkIndexReader.cssrc/Elastic.Documentation.Links/CrossLinks/CrossLinkFetcher.cssrc/Elastic.Documentation/Elastic.Documentation.csprojsrc/Elastic.Documentation/IDocumentationContext.cssrc/Elastic.Markdown/Myst/Directives/CsvInclude/CsvReader.cssrc/authoring/Elastic.Documentation.Refactor/FormatService.cssrc/authoring/Elastic.Documentation.Refactor/MoveFileService.cssrc/authoring/Elastic.Documentation.Refactor/Tracking/LocalChangesService.cssrc/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cssrc/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cssrc/services/Elastic.Changelog/Bundling/ChangelogRemoveService.cssrc/services/Elastic.Changelog/Bundling/ProfileFilterResolver.cssrc/services/Elastic.Changelog/Bundling/PromotionReportParser.cssrc/services/Elastic.Changelog/Creation/ChangelogCreationService.cssrc/services/Elastic.Changelog/Evaluation/ChangelogPrEvaluationService.cssrc/services/Elastic.Changelog/GithubRelease/GitHubReleaseChangelogService.cssrc/services/Elastic.Changelog/Rendering/ChangelogRenderer.cssrc/services/Elastic.Changelog/Rendering/ChangelogRenderingService.cssrc/services/Elastic.Changelog/Rendering/Markdown/BreakingChangesMarkdownRenderer.cssrc/services/Elastic.Changelog/Rendering/Markdown/ChangelogMarkdownRenderer.cssrc/services/Elastic.Changelog/Rendering/Markdown/DeprecationsMarkdownRenderer.cssrc/services/Elastic.Changelog/Rendering/Markdown/HighlightsMarkdownRenderer.cssrc/services/Elastic.Changelog/Rendering/Markdown/IndexMarkdownRenderer.cssrc/services/Elastic.Changelog/Rendering/Markdown/KnownIssuesMarkdownRenderer.cssrc/services/Elastic.Changelog/Rendering/Markdown/MarkdownRendererBase.cssrc/services/Elastic.Documentation.Assembler/AssembleContext.cssrc/services/Elastic.Documentation.Assembler/Building/AssemblerBuildService.cssrc/services/Elastic.Documentation.Assembler/Building/AssemblerSitemapService.cssrc/services/Elastic.Documentation.Assembler/ContentSources/RepositoryBuildMatchingService.cssrc/services/Elastic.Documentation.Assembler/ContentSources/RepositoryPublishValidationService.cssrc/services/Elastic.Documentation.Assembler/Deploying/IncrementalDeployService.cssrc/services/Elastic.Documentation.Assembler/Elastic.Documentation.Assembler.csprojsrc/services/Elastic.Documentation.Assembler/Indexing/AssemblerIndexService.cssrc/services/Elastic.Documentation.Assembler/Navigation/GlobalNavigationService.cssrc/services/Elastic.Documentation.Isolated/IsolatedBuildService.cssrc/services/Elastic.Documentation.Isolated/IsolatedIndexService.cssrc/tooling/docs-builder/Http/DocumentationWebHost.cssrc/tooling/docs-builder/Http/InMemoryBuildState.cstests-integration/Elastic.Assembler.IntegrationTests/AssemblerConfigurationTests.cstests-integration/Elastic.Assembler.IntegrationTests/DocsSyncTests.cstests-integration/Elastic.Assembler.IntegrationTests/NavigationBuildingTests.cstests-integration/Elastic.Assembler.IntegrationTests/NavigationRootTests.cstests-integration/Elastic.Assembler.IntegrationTests/SiteNavigationTests.cstests/Elastic.ApiExplorer.Tests/Elastic.ApiExplorer.Tests.csprojtests/Elastic.ApiExplorer.Tests/ReaderTests.cstests/Elastic.Changelog.Tests/Changelogs/BundleChangelogsTests.cstests/Elastic.Changelog.Tests/Changelogs/ChangelogRemoveTests.cstests/Elastic.Changelog.Tests/Changelogs/ChangelogTestBase.cstests/Elastic.Changelog.Tests/Elastic.Changelog.Tests.csprojtests/Elastic.Documentation.Configuration.Tests/CrossLinkRegistryTests.cstests/Elastic.Documentation.Configuration.Tests/DocumentationSetFileTests.cstests/Elastic.Markdown.Tests/Assembler/AssemblerHtmxMarkdownLinkTests.cstests/Elastic.Markdown.Tests/Codex/CodexHtmxCrossLinkTests.cstests/Elastic.Markdown.Tests/Directives/CsvIncludeTests.cstests/Elastic.Markdown.Tests/Directives/DirectiveBaseTests.cstests/Elastic.Markdown.Tests/DocSet/NavigationTestsBase.cstests/Elastic.Markdown.Tests/Elastic.Markdown.Tests.csprojtests/Elastic.Markdown.Tests/Inline/ImagePathResolutionTests.cstests/Elastic.Markdown.Tests/Inline/InlneBaseTests.cstests/Elastic.Markdown.Tests/OutputDirectoryTests.cstests/Elastic.Markdown.Tests/RootIndexValidationTests.cstests/Navigation.Tests/Assembler/ComplexSiteNavigationTests.cstests/Navigation.Tests/Assembler/IdentifierCollectionTests.cstests/Navigation.Tests/Assembler/SiteDocumentationSetsTests.cstests/Navigation.Tests/Assembler/SiteNavigationTests.cstests/Navigation.Tests/Codex/CodexNavigationTestBase.cstests/Navigation.Tests/Codex/GroupNavigationTests.cstests/Navigation.Tests/Isolation/PhysicalDocsetTests.cstests/Navigation.Tests/Navigation.Tests.csprojtests/Navigation.Tests/TestDocumentationSetContext.cstests/authoring/Framework/CrossLinkResolverAssertions.fstests/authoring/Framework/Setup.fs
✅ Files skipped from review due to trivial changes (9)
- src/Elastic.Documentation/Elastic.Documentation.csproj
- src/services/Elastic.Documentation.Assembler/Elastic.Documentation.Assembler.csproj
- tests/Elastic.ApiExplorer.Tests/Elastic.ApiExplorer.Tests.csproj
- tests/Elastic.Markdown.Tests/Codex/CodexHtmxCrossLinkTests.cs
- tests/Navigation.Tests/Navigation.Tests.csproj
- tests/Elastic.Changelog.Tests/Elastic.Changelog.Tests.csproj
- src/tooling/docs-builder/Http/InMemoryBuildState.cs
- tests/Navigation.Tests/Isolation/PhysicalDocsetTests.cs
- tests/Elastic.Markdown.Tests/Elastic.Markdown.Tests.csproj
🚧 Files skipped from review as they are similar to previous changes (16)
- src/services/Elastic.Documentation.Assembler/Indexing/AssemblerIndexService.cs
- src/services/Elastic.Documentation.Assembler/Deploying/IncrementalDeployService.cs
- src/services/Elastic.Documentation.Assembler/ContentSources/RepositoryBuildMatchingService.cs
- src/authoring/Elastic.Documentation.Refactor/Tracking/LocalChangesService.cs
- src/services/Elastic.Documentation.Assembler/Building/AssemblerSitemapService.cs
- src/services/Elastic.Documentation.Isolated/IsolatedBuildService.cs
- src/services/Elastic.Changelog/Bundling/ChangelogBundlingService.cs
- src/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cs
- src/Elastic.Markdown/Myst/Directives/CsvInclude/CsvReader.cs
- src/services/Elastic.Changelog/Bundling/PromotionReportParser.cs
- src/services/Elastic.Changelog/GithubRelease/GitHubReleaseChangelogService.cs
- src/services/Elastic.Changelog/Bundling/ChangelogRemoveService.cs
- src/Elastic.Documentation.Configuration/FileSystemFactory.cs
- src/Elastic.Documentation.LinkIndex/GitLinkIndexReader.cs
- src/services/Elastic.Changelog/Evaluation/ChangelogPrEvaluationService.cs
- src/services/Elastic.Changelog/Creation/ChangelogCreationService.cs
src/services/Elastic.Changelog/Rendering/ChangelogRenderingService.cs
Outdated
Show resolved
Hide resolved
tests-integration/Elastic.Assembler.IntegrationTests/NavigationBuildingTests.cs
Outdated
Show resolved
Hide resolved
RealRead/RealWrite are pre-allocated singletons scoped to Paths.WorkingDirectoryRoot (the CWD git root). Commands that accept an explicit --path or --output argument may target a repo outside that root, which would cause ScopedFileSystemException. Add FileSystemFactory.ForPath(string? path) and ForPathWrite(string? path, string? output) that derive the scope root dynamically by walking up from the given path to find its .git boundary. Both fall back to RealRead/RealWrite when no path is provided (CWD-relative operation). ForPathWrite also adds the output directory as an extra scope root when it falls outside the git root. Update all commands that accept --path/--output to use these: IndexCommand, IsolatedBuildCommand, ServeCommand, FormatCommand, MoveCommand, DiffCommands, InMemoryBuildState, StaticWebHost. Commands that always operate relative to CWD (assembler, codex, changelog) continue using RealRead since their scope is determined by assembler config or git context, not an arbitrary user-provided path. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Three changes: 1. FileSystemFactory.FindGitRoot removed — ForPath/ForPathWrite now call Paths.FindGitRoot, which is the single canonical git-root walker. 2. Paths.DetermineSourceDirectoryRoot simplified — the loop body had a dead branch (checked GetDirectories.Length == 0 in the while condition then re-checked inside). Rewritten to a clean while-loop matching FindGitRoot's structure; semantics unchanged. 3. Paths.GitCommonRoot static field and InitGitCommonRoot removed — the static ran complex logic (ScopedFileSystem creation + file read) at class init time and was only used by AssembleContext and CodexContext. Both now call Paths.ResolveGitCommonRoot(readFileSystem, workingRoot) directly, which is already public and takes explicit dependencies. Paths.cs no longer references Nullean.ScopedFileSystem. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Move checkout directories to ApplicationData:
- AssembleContext: .artifacts/checkouts/{env} → ApplicationData/checkouts/{env}
- CodexContext: .artifacts/codex/clone → ApplicationData/codex/clone
Both now resolve within the existing ApplicationData scope root; no
worktree detection or scope expansion needed. Paths.ResolveGitCommonRoot
(IFileSystem) had no remaining callers and is removed along with its tests.
Depth protection on git root discovery:
- DetermineWorkingDirectoryRoot: only adopts a .git anchor ≤ 1 directory
above CWD in release builds. Debug builds allow deeper traversal when
a *.slnx is adjacent (developer running from IDE output directory).
- FindGitRoot(string) gets the same depth limit — documentation is not
expected to live deep inside a repo tree.
Consolidate FindGitRoot / DetermineSourceDirectoryRoot:
- DetermineSourceDirectoryRoot removed; FindGitRoot(IDirectoryInfo)
overload added with the same semantics (IFileSystem-abstracted,
same depth protection, returns IDirectoryInfo?).
- Callers updated: BuildContext, ChangelogCommand, LocalChangesService.
Rename ForPath/ForPathWrite → RealForPath/RealForPathWrite:
- Signals these always create a real FileSystem.
- Doc comment notes suitability for command layer; service layer is
tested via InMemory() at unit test level.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
WriteOptions had AllowedSpecialFolders.Temp removed on the assumption that only ConfigurationFileProvider used it. AwsS3SyncApplyStrategy also uses temp to stage files before S3 upload — restore Temp to WriteOptions to fix the DocsSyncTests.TestApply integration failure. Fix IMPORTS lint errors across all files that gained new Nullean.ScopedFileSystem using directives during the ScopedFileSystem migration; dotnet format reorders them into the expected alphabetical/grouped order. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Three issues: 1. Changelog tests used FileSystem.Path.GetTempPath() to generate unique paths within MockFileSystem. Since MockFS is in-memory, the paths can be anything in scope — replace with Paths.WorkingDirectoryRoot.FullName as the base so they stay within the ScopedFileSystem scope bounds. No real filesystem is touched; all operations remain in-memory. 2. DocsSyncTests.TestApply passed the same WrapToRead FS for both read and write in AssembleContext. AwsS3SyncApplyStrategy uses context.WriteFileSystem and needs AllowedSpecialFolders.Temp (present in WriteOptions). Fix by using WrapToWrite for the write FS. 3. IsolatedBuildService CI fallback defaulted the output path to Paths.WorkingDirectoryRoot/.artifacts/docs/html. When --path points to a different repo the write FS is scoped to that repo's root, not WorkingDirectoryRoot. Derive the default output from `path` instead so it stays within the write FS scope (same logic as BuildContext uses for the normal build path). FileSystemFactory.RealForPathWrite: replace StartsWith string check with IDirectoryInfo.IsSubPathOf from Nullean.ScopedFileSystem which does a proper directory-tree walk, preventing sibling-prefix false positives. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
CsvReader: remove GetType().Name == "MockFileSystem" type check — the check never matched after ScopedFileSystem wrapping was introduced, causing Sep to read from the real filesystem via FromFile() instead of going through the scoped IFileSystem. Always use IFileSystem.File.ReadAllText + Sep.FromText so all filesystem access (real, scoped, or mock) goes through the abstraction. ChangelogCommand: four ChangelogConfigurationLoader instantiations were still passing new System.IO.Abstractions.FileSystem() directly, bypassing the scoped _fileSystem field. Replace with _fileSystem. ChangelogRenderingService: service writes output files (CreateDirectory, WriteAllTextAsync) so its default should be FileSystemFactory.RealWrite not RealRead. RealWrite has the same scope but correctly excludes .git write access. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…rkingDirectoryRoot-relative
Tests that extend ChangelogTestBase use a ScopedFileSystem wrapping MockFileSystem,
bounded to [WorkingDirectoryRoot, ApplicationData]. Several tests used absolute paths
like /tmp/config, /docs/changelog, /test-root, and relative paths like docs/changelog
that resolved outside the scope root on CI runners.
- ChangelogPrEvaluationServiceTests: /tmp/config/changelog.yml →
Path.Join(WorkingDirectoryRoot, "config/changelog.yml"); "docs/changelog" →
Path.Join(WorkingDirectoryRoot, "docs/changelog")
- ChangelogCreationServiceTests: /tmp/config → Path.Join(WorkingDirectoryRoot, "config");
/tmp/output → Path.Join(WorkingDirectoryRoot, "output")
- BundleChangelogsTests: /test-root → WorkingDirectoryRoot; use MockFileSystemOptions
{ CurrentDirectory = root } so relative paths within service code resolve correctly;
switch YAML template config strings to $$""" raw literals so {version}/{lifecycle}
stay as literal text while {{Path.Join(...)}} is interpolated
- ChangelogTestBase: set MockFileSystem CurrentDirectory to WorkingDirectoryRoot so
paths within the test base resolve within scope
BundleLoaderTests (standalone, unscoped MockFileSystem) does not extend ChangelogTestBase
and is not affected.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…ion test scope WrapToRead/WrapToWrite implicitly injected Paths.WorkingDirectoryRoot into every scope, which was a wrong assumption when the inner FS contained files outside the working directory (e.g. navigation tests with MockFS at /checkouts/...). New explicit API in FileSystemFactory: - ScopeCurrentWorkingDirectory(inner) — scopes to WorkingDirectoryRoot + ApplicationData - ScopeCurrentWorkingDirectory(inner, extensionRoots) — adds detection-rules roots - ScopeCurrentWorkingDirectoryForWrite(inner) — write variant (no .git) - ScopeSourceDirectory(inner, sourceRoot) — scopes to an explicit root + ApplicationData - ScopeSourceDirectoryForWrite(inner, sourceRoot) — write variant All production options fields renamed: ReadOptions → WorkingDirectoryReadOptions, WriteOptions → WorkingDirectoryWriteOptions (makes scope intent visible in code). DocumentationSetFile.LoadAndResolve: replace the InvalidOperationException cast guard with FileSystemFactory.ScopeSourceDirectory(fs, directory.FullName) so callers with unscoped IFileSystem/IDirectoryInfo (e.g. from a raw MockFileSystem) are automatically given the tightest correct scope. Navigation.Tests/Assembler: change ScopeCurrentWorkingDirectory(fileSystem) → ScopeSourceDirectory(fileSystem, "/checkouts") for all tests that use SiteNavigationTestFixture (MockFS rooted at /checkouts/current/...). TestDocumentationSetContext: use ScopeSourceDirectory(fileSystem, sourceDirectory.FullName) and ScopeSourceDirectoryForWrite(fileSystem, outputDirectory.FullName) so each test gets a scope precisely matching its mock FS tree. Fix 3 semantic bugs where WriteFileSystem was assigned ScopeCurrentWorkingDirectory instead of ScopeCurrentWorkingDirectoryForWrite: - CodexNavigationTestBase, GroupNavigationTests, CrossLinkRegistryTests Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
RealForPath → RealGitRootForPath RealForPathWrite → RealGitRootForPathWrite The Git prefix makes explicit that these factory methods resolve the scope root by walking up to the nearest .git boundary from the given path. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR introduces
Nullean.ScopedFileSystemas a security boundary around every file system operation in the codebase.What is
ScopedFileSystem?ScopedFileSystemis aSystem.IO.Abstractions.IFileSystemdecorator that enforces at runtime that no file read or write can escape a configured set of root directories. Any operation targeting a path outside those roots — including paths reached via..traversal, symlink, or hidden directory — throws aScopedFileSystemException(extendsSecurityException) before the underlying OS call is made.File.Exists/Directory.Existsreturnfalsefor out-of-scope paths rather than throwing, so probing code stays safe without additional guards.The library supports:
.git)Temp,LocalApplicationData)IFileSystemimplementation includingMockFileSystemChanges
FileSystemFactoryinElastic.Documentation.Configuration:FileSystemFactory.Real— pre-allocated singleton scoped to the working-directory root +Paths.ApplicationData(LocalApplicationData/elastic/docs-builder), with.gitfolder/file allowlisted andTempenabledFileSystemFactory.InMemory()— wraps a freshMockFileSystemin the same options; each call returns a new independent in-memory instanceFileSystemFactory.CreateScoped(IFileSystem inner, IEnumerable<string>?)— wraps any FS with optional additional roots (used for plugin extension roots)FileSystemFactory.CreateForUserData()— user-profile scope with.docs-builderallowlisted, used byGitLinkIndexReader(~/.docs-builder/codex-link-index)new FileSystem()call sites replaced with factory methods;MockFileSysteminstances wrapped withFileSystemFactory.CreateScoped(inner)CrossLinkFetcherandCheckForUpdatesFilterconverted from directSystem.IO.File/Directorystatic calls toIFileSysteminjectionIDocsBuilderExtensiongainsExternalScopeRoots(default[]);DetectionRulesDocsBuilderExtensionimplements it to expose detection-rules folder paths so builds that reference rules outside the workspace can widen the scopeIFileSystemregistered as DI singleton (FileSystemFactory.Real) inDocumentationToolingFileSystemtype widened toIFileSystemTest plan
dotnet buildpasses (verified)docs-builder buildcompletes withoutScopedFileSystemExceptiondocs-builder servestarts and serves docsdocs-builder build --in-memoryworks (usesFileSystemFactory.InMemory())🤖 Generated with Claude Code