Repository navigation
Conversation
`mkdir` never touched `write_bytes_limit`, so a mount with a write budget could still be filled with unlimited directories. Direct and overlay mounts now charge `DIRECTORY_WRITE_BYTES` (4,096) per directory created, parents included, checked before anything is created and committed after. Overlay `mkdir(parents=True)` collects the directories it will record first, so a refused call records none of them; read-only mounts already refuse mkdir. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Branch diff:
|
| Commit | Message | Production | Test | Docs | Other |
|---|---|---|---|---|---|
| 56b273b | Charge directory creation aga… | +75 -43 | +69 -1 | +4 -0 | +0 -0 |
| TOTAL | +75 -43 | +69 -1 | +4 -0 | +0 -0 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
4 issues found across 9 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/monty-fs/src/direct.rs">
<violation number="1" location="crates/monty-fs/src/direct.rs:202">
P3: The quota check runs before `host_mkdir` validates the request, so a `mkdir` that would fail for another reason can report `WriteLimitExceeded` instead. Consider running the path checks first, or accept that the quota error takes precedence and document it.</violation>
<violation number="2" location="crates/monty-fs/src/direct.rs:218">
P2: `missing_dirs` treats a dangling symlink as an absent directory and can reject `mkdir` on that existing entry with `WriteLimitExceeded` before `host_mkdir` reports `FileExistsError`. Use a no-follow existence check when determining which entries are actually missing.</violation>
</file>
<file name="crates/monty-fs/src/overlay.rs">
<violation number="1" location="crates/monty-fs/src/overlay.rs:884">
P2: The per-directory `state.insert` loop can leave a partial tree when the overlay memory limit rejects a later entry, while the write charge is not committed. Preflight the complete replacement set and insert it atomically, or roll back earlier entries on failure; otherwise a retry can undercharge directories that were already recorded.</violation>
</file>
<file name="docs/limitations/filesystem.md">
<violation number="1" location="docs/limitations/filesystem.md:128">
P3: The new sentence says the 4,096-byte charge is taken 'before anything is created', but the code only checks the budget first and records the charge after `mkdir` succeeds. Also, 'would pass the limit' is ambiguous; say 'would exceed the limit'.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
| .find(|&n| absent(&components[..n].join("/"))) | ||
| .map_or(0, |n| components.len() + 1 - n) | ||
| } else { | ||
| usize::from(absent(rel)) |
Contributor
There was a problem hiding this comment.
P2: missing_dirs treats a dangling symlink as an absent directory and can reject mkdir on that existing entry with WriteLimitExceeded before host_mkdir reports FileExistsError. Use a no-follow existence check when determining which entries are actually missing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At crates/monty-fs/src/direct.rs, line 218:
<comment>`missing_dirs` treats a dangling symlink as an absent directory and can reject `mkdir` on that existing entry with `WriteLimitExceeded` before `host_mkdir` reports `FileExistsError`. Use a no-follow existence check when determining which entries are actually missing.</comment>
<file context>
@@ -188,9 +188,35 @@ fn append_bytes(path: &str, data: &[u8], ctx: &mut MountContext<'_>) -> Result<M
+ .find(|&n| absent(&components[..n].join("/")))
+ .map_or(0, |n| components.len() + 1 - n)
+ } else {
+ usize::from(absent(rel))
+ }
}
</file context>
Suggested change
| usize::from(absent(rel)) | |
| usize::from(dir.symlink_metadata(rel).is_err()) |
| let charge = to_create.len().saturating_mul(DIRECTORY_WRITE_BYTES); | ||
| check_write_limit(charge, ctx)?; | ||
| for dir in to_create { | ||
| state.insert( |
Contributor
There was a problem hiding this comment.
P2: The per-directory state.insert loop can leave a partial tree when the overlay memory limit rejects a later entry, while the write charge is not committed. Preflight the complete replacement set and insert it atomically, or roll back earlier entries on failure; otherwise a retry can undercharge directories that were already recorded.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At crates/monty-fs/src/overlay.rs, line 884:
<comment>The per-directory `state.insert` loop can leave a partial tree when the overlay memory limit rejects a later entry, while the write charge is not committed. Preflight the complete replacement set and insert it atomically, or roll back earlier entries on failure; otherwise a retry can undercharge directories that were already recorded.</comment>
<file context>
@@ -870,30 +871,42 @@ fn mkdir(
+ let charge = to_create.len().saturating_mul(DIRECTORY_WRITE_BYTES);
+ check_write_limit(charge, ctx)?;
+ for dir in to_create {
+ state.insert(
+ dir,
+ OverlayEntry::Directory {
</file context>
| `OverlayMemory`, appending to an existing real file can materialize that | ||
| file into the in-memory overlay, so the existing file bytes count against | ||
| the limit along with the newly appended bytes. | ||
| Each directory `mkdir` creates, parents included, is charged 4,096 bytes against the limit before anything is |
Contributor
There was a problem hiding this comment.
P3: The new sentence says the 4,096-byte charge is taken 'before anything is created', but the code only checks the budget first and records the charge after mkdir succeeds. Also, 'would pass the limit' is ambiguous; say 'would exceed the limit'.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/limitations/filesystem.md, line 128:
<comment>The new sentence says the 4,096-byte charge is taken 'before anything is created', but the code only checks the budget first and records the charge after `mkdir` succeeds. Also, 'would pass the limit' is ambiguous; say 'would exceed the limit'.</comment>
<file context>
@@ -125,6 +125,8 @@ Hosts can configure a cumulative `write_bytes_limit` per mount. In
`OverlayMemory`, appending to an existing real file can materialize that
file into the in-memory overlay, so the existing file bytes count against
the limit along with the newly appended bytes.
+Each directory `mkdir` creates, parents included, is charged 4,096 bytes against the limit before anything is
+created, so a `mkdir(parents=True)` that would pass the limit creates nothing.
</file context>
| } else { | ||
| 0 | ||
| }; | ||
| check_write_limit(charge, ctx)?; |
Contributor
There was a problem hiding this comment.
P3: The quota check runs before host_mkdir validates the request, so a mkdir that would fail for another reason can report WriteLimitExceeded instead. Consider running the path checks first, or accept that the quota error takes precedence and document it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At crates/monty-fs/src/direct.rs, line 202:
<comment>The quota check runs before `host_mkdir` validates the request, so a `mkdir` that would fail for another reason can report `WriteLimitExceeded` instead. Consider running the path checks first, or accept that the quota error takes precedence and document it.</comment>
<file context>
@@ -188,9 +188,35 @@ fn append_bytes(path: &str, data: &[u8], ctx: &mut MountContext<'_>) -> Result<M
+ } else {
+ 0
+ };
+ check_write_limit(charge, ctx)?;
+ let result = host_mkdir(ctx.mount_dir, rel, parents, exist_ok, path)?;
+ commit_write_bytes(charge, ctx);
</file context>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
Merging this PR will regress 5 benchmarks1 warning
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing 
Footnotes
|
This branch has not been deployed
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
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.
We can optionally (default off) set write limits on a mount, however we do not currently charge for writing directories.
This prices directories at 4096 bytes charging them towards the same
write_bytes_limit.🤖 Generated with Claude Code
Summary by cubic
Makes
mkdircount against a mount'swrite_bytes_limitso a write budget also bounds how many directories sandboxed code can create.DIRECTORY_WRITE_BYTES) before anything is created; the charge is committed only after the operation succeeds.mkdir(parents=True)collects the directories it will record first, so a refused call leaves no partial tree.mkdiron an existing directory is free, and the limit remains optional with unlimited as the default.Written for commit 56b273b. Summary will update on new commits.