Skip to content

Use strings.Compare in readDir sort#5

Merged
AlexandreYang merged 1 commit into
mainfrom
dd/use-strings-compare-readdir
Mar 9, 2026
Merged

Use strings.Compare in readDir sort#5
AlexandreYang merged 1 commit into
mainfrom
dd/use-strings-compare-readdir

Conversation

@AlexandreYang

Copy link
Copy Markdown
Member

Summary

Simplify the sort comparator in readDir by replacing manual -1/0/1 branching with strings.Compare.

Changes

  • Replaced two-if-branch comparator in interp/allowed_paths.go with a single strings.Compare(a.Name(), b.Name()) call.

Testing

  • go build ./interp/... passes.

PR by Bits
View session in Datadog

Comment @DataDog to request changes

@datadog-datadog-prod-us1

Copy link
Copy Markdown

View session in Datadog

Bits Dev status: ✅ Done

CI Auto-fix: Disabled | Enable

Comment @DataDog to request changes

@AlexandreYang
AlexandreYang merged commit 1b0d362 into main Mar 9, 2026
5 of 7 checks passed
@AlexandreYang
AlexandreYang deleted the dd/use-strings-compare-readdir branch March 9, 2026 09:28
This was referenced Mar 12, 2026
julesmcrt added a commit that referenced this pull request May 12, 2026
…cs; close coverage gaps

Four review items from @AlexandreYang on PR #237 addressed in one
commit:

1. Promote the -P-above-sandbox passthrough invariant from a local
   comment into the package docstring so callers can reason about
   when cd -P diverges from bash on container-style sandboxes.

2. Improve the diagnostic emitted when ChangeDir is nil (the
   RunCommand-child case used by find -exec / -execdir / xargs).
   Was "cd: not supported in this runner" — generic and opaque to
   users. Now reads "cd: cannot change directory from inside
   find -exec/-execdir or xargs (child invocations are isolated)"
   so the user knows the cause is intentional isolation.

3. Add a scenario for HOME pointing outside the sandbox
   (errors/home_outside_sandbox.yaml). Existing scenarios only
   covered HOME unset and HOME empty; the rejection path where
   resolveOperand succeeds but changeDir rejects was untested.

4. Add four hardening unit tests for the HostPrefix re-prefix
   logic in resolveSymlinks: unrelated absolute target gets
   re-prefixed, already-prefixed target is left alone, target
   exactly equal to hp is left alone, and the
   /mnt/host vs /mnt/hostile prefix-collision boundary is
   correctly distinguished by the hp+sep check.

P3 review items (#5 Windows ACL test, #6 isCd literal-name
defer-skip) are intentionally not addressed — both are flagged
as not blocking and are forward-looking notes; replies posted
on the threads.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants