Repository navigation
Merged
Conversation
…ed for a symlink mid-walk (D-1161)
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.
Rows
D-0450 — ALREADY-FIXED, no change.
addPluginCoreEnvVars(internal/build/plugins_core_env.go) writesS3_ENDPOINT/S3_REGIONfor the active core object store (SeaweedFS, or MinIO on its configured port) into.nself/compose.env; landed in 3d070ba (#809). Covered byinternal/build/compose_env_object_storage_test.go(passes on this branch). The storage plugin's ownminio:9000default is now only a no-store fallback.D-0777 — BLOCKED, no change. The preferred resolution needs a product decision (a manifest field declaring a public route or auth gate, and the default-deny semantics for plugin routes) plus changes in the plugins repo (gating or exempting the 38 ungated plugins, the ratchet test). Neither is local to this repo. Today
pluginSnippetSkipstill refuses bare top-levellocationsnippets, so nothing is newly exposed.D-1161 — FIXED.
removeTreerecursed with a path-basedLstatfollowed byReadDir/Remove, so a directory swapped for a symlink in between redirected the deletion. It now carries each directory'sLstatidentity and re-checks (os.SameFileon a non-followingLstat) after the listing and before every removal, aborting witherrTreeSwappedinstead of deleting through the link. This narrows the window to the instant between the check and the unlink; it does not make the walk fd-based, since the Sink interface is path-based. The review report named in the row was not available to me, so I fixed the described TOCTOU only. Nested marker handling is unchanged (a nested.nself-generatedis still removed last in its own directory, and plan and apply walk the same way).New test
TestProxyRestoreDirSwappedForSymlinkMidWalkswaps a subdirectory for a symlink to a victim directory as the walk lists it. Without the fix it returns nil and deletes through the link; with it the cleanup stops and the victim is untouched.Commands
go build ./...OKgo vet ./internal/buildOKgo test ./internal/build -count=1ok (24s)proxy_restore.go(with a stub for the new error var): FAIL as expected; with the fix: PASS.