Skip to content

fix(execd): validate request.cwd and return 400#656

Merged
hittyt merged 4 commits intoalibaba:mainfrom
Pangjiping:hotfix/execd/missing-cwd
Apr 9, 2026
Merged

fix(execd): validate request.cwd and return 400#656
hittyt merged 4 commits intoalibaba:mainfrom
Pangjiping:hotfix/execd/missing-cwd

Conversation

@Pangjiping
Copy link
Copy Markdown
Collaborator

Summary

  • validate request.cwd and return 400

Testing

  • Not run (explain why)
  • Unit tests
  • Integration tests
  • e2e / manual verification

Breaking Changes

  • None
  • Yes (describe impact and migration path)

Checklist

  • Linked Issue or clearly described motivation
  • Added/updated docs (if needed)
  • Added/updated tests (if needed)
  • Security impact considered
  • Backward compatibility considered

@Pangjiping Pangjiping added bug Something isn't working component/execd labels Apr 8, 2026
@Pangjiping Pangjiping force-pushed the hotfix/execd/missing-cwd branch from dc994db to fa97875 Compare April 8, 2026 02:26
@Pangjiping Pangjiping requested a review from jwx0925 as a code owner April 8, 2026 02:26
@ninan-nn
Copy link
Copy Markdown
Collaborator

ninan-nn commented Apr 9, 2026

The new cwd validation looks good for RunCommandRequest and RunInSessionRequest. One thing I wanted to double-check is the behavior of the other cwd entrypoints: CreateContext currently auto-creates the target directory via os.MkdirAll, so it does not need to follow the same validation path. But CreateSession / CreatePTYSession do not create the directory for the caller; they just carry the provided cwd forward and will fail later when the process starts. If we want invalid request.cwd to be handled consistently at the API boundary, those entrypoints should be validated as well and return 400 instead of surfacing as a later runtime failure.

@Pangjiping
Copy link
Copy Markdown
Collaborator Author

CreatePTYSession

Great point! I previously overlooked the cwd setup in CreateSession and CreatePTYSession. I’ve now aligned the creation flow to set up cwd as well, following a fast-fail strategy.

@Pangjiping Pangjiping force-pushed the hotfix/execd/missing-cwd branch from 67c1791 to cc21a42 Compare April 9, 2026 07:41
@Pangjiping Pangjiping force-pushed the hotfix/execd/missing-cwd branch from cc21a42 to 184d2d0 Compare April 9, 2026 07:49
Copy link
Copy Markdown
Collaborator

@hittyt hittyt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hittyt hittyt merged commit 9766b40 into alibaba:main Apr 9, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working component/execd

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants