99 "strings"
1010)
1111
12- // TmpStorage persists and retrieves artifact files.
13- type TmpStorage interface {
14- // Put copies a local file to tmp storage at the given key.
12+ // Storage persists and retrieves artifact files.
13+ type Storage interface {
14+ // Put copies a local file to storage at the given key.
1515 // Returns the URL or path where the file can be accessed.
1616 Put (ctx context.Context , key string , localPath string ) (string , error )
1717
@@ -22,32 +22,39 @@ type TmpStorage interface {
2222 DeleteByPrefix (ctx context.Context , prefix string ) error
2323}
2424
25- // FilesystemTmpStorage stores artifacts on the local filesystem.
26- type FilesystemTmpStorage struct {
25+ // FilesystemStorage stores artifacts on the local filesystem.
26+ type FilesystemStorage struct {
2727 BasePath string
2828}
2929
3030// Put copies the file at localPath to the storage location identified by key and returns its path.
31- func (fs * FilesystemTmpStorage ) Put (ctx context.Context , key string , localPath string ) (string , error ) {
31+ func (fs * FilesystemStorage ) Put (ctx context.Context , key string , localPath string ) (string , error ) {
32+ // Reject obviously malicious keys before any filesystem operations.
33+ if filepath .IsAbs (key ) || key == ".." || strings .HasPrefix (key , ".." + string (filepath .Separator )) {
34+ return "" , fmt .Errorf ("invalid artifact key: must be a relative path within the storage directory" )
35+ }
36+
3237 destPath := filepath .Join (fs .BasePath , key )
3338 // Validate destination is within BasePath to prevent path traversal.
34- absBase , err := filepath .Abs (fs .BasePath )
39+ // Use EvalSymlinks for canonicalization to prevent symlink-based escapes.
40+ resolvedBase , err := filepath .EvalSymlinks (fs .BasePath )
3541 if err != nil {
3642 return "" , fmt .Errorf ("resolving base path: %w" , err )
3743 }
38- absDest , err := filepath .Abs (destPath )
44+ // Ensure dest directory exists before resolving symlinks on the full path.
45+ if err := os .MkdirAll (filepath .Dir (destPath ), 0755 ); err != nil {
46+ return "" , fmt .Errorf ("creating artifact dir: %w" , err )
47+ }
48+ resolvedDest , err := filepath .EvalSymlinks (filepath .Dir (destPath ))
3949 if err != nil {
4050 return "" , fmt .Errorf ("resolving dest path: %w" , err )
4151 }
42- rel , err := filepath .Rel (absBase , absDest )
52+ resolvedDest = filepath .Join (resolvedDest , filepath .Base (destPath ))
53+ rel , err := filepath .Rel (resolvedBase , resolvedDest )
4354 if err != nil || strings .HasPrefix (rel , ".." ) {
4455 return "" , fmt .Errorf ("key escapes base path" )
4556 }
4657
47- if err := os .MkdirAll (filepath .Dir (destPath ), 0755 ); err != nil {
48- return "" , fmt .Errorf ("creating artifact dir: %w" , err )
49- }
50-
5158 // Reject symlinks and non-regular files to prevent exfiltration.
5259 fi , err := os .Lstat (localPath )
5360 if err != nil {
@@ -79,38 +86,57 @@ func (fs *FilesystemTmpStorage) Put(ctx context.Context, key string, localPath s
7986}
8087
8188// DeleteByPrefix removes all files stored under the given key prefix.
82- func (fs * FilesystemTmpStorage ) DeleteByPrefix (ctx context.Context , prefix string ) error {
89+ func (fs * FilesystemStorage ) DeleteByPrefix (ctx context.Context , prefix string ) error {
8390 target := filepath .Join (fs .BasePath , prefix )
84- absBase , err := filepath .Abs (fs .BasePath )
91+ resolvedBase , err := filepath .EvalSymlinks (fs .BasePath )
8592 if err != nil {
8693 return fmt .Errorf ("resolving base path: %w" , err )
8794 }
88- absTarget , err := filepath .Abs (target )
95+ resolvedTarget , err := filepath .EvalSymlinks (target )
8996 if err != nil {
97+ // Target doesn't exist — compute candidate relative to resolvedBase for traversal check.
98+ candidate := filepath .Join (resolvedBase , prefix )
99+ rel , relErr := filepath .Rel (resolvedBase , candidate )
100+ if relErr != nil || strings .HasPrefix (rel , ".." ) {
101+ return fmt .Errorf ("prefix escapes base path" )
102+ }
103+ // Target doesn't exist and is within base — nothing to delete.
104+ if os .IsNotExist (err ) {
105+ return nil
106+ }
90107 return fmt .Errorf ("resolving target path: %w" , err )
91108 }
92- rel , err := filepath .Rel (absBase , absTarget )
109+ rel , err := filepath .Rel (resolvedBase , resolvedTarget )
93110 if err != nil || strings .HasPrefix (rel , ".." ) {
94111 return fmt .Errorf ("prefix escapes base path" )
95112 }
96- return os .RemoveAll (target )
113+ return os .RemoveAll (resolvedTarget )
97114}
98115
99116// Delete removes a single artifact file by its path (as returned by Put).
100- func (fs * FilesystemTmpStorage ) Delete (ctx context.Context , url string ) error {
101- absBase , err := filepath .Abs (fs .BasePath )
117+ func (fs * FilesystemStorage ) Delete (ctx context.Context , url string ) error {
118+ resolvedBase , err := filepath .EvalSymlinks (fs .BasePath )
102119 if err != nil {
103120 return fmt .Errorf ("resolving base path: %w" , err )
104121 }
105- absURL , err := filepath .Abs (url )
122+ resolvedURL , err := filepath .EvalSymlinks (url )
106123 if err != nil {
124+ // File doesn't exist — compute path relative to resolvedBase for traversal check.
125+ // url is an absolute path returned by Put, so use it directly.
126+ rel , relErr := filepath .Rel (resolvedBase , url )
127+ if relErr != nil || strings .HasPrefix (rel , ".." ) {
128+ return fmt .Errorf ("artifact path escapes base path" )
129+ }
130+ if os .IsNotExist (err ) {
131+ return nil // already gone
132+ }
107133 return fmt .Errorf ("resolving artifact path: %w" , err )
108134 }
109- rel , err := filepath .Rel (absBase , absURL )
135+ rel , err := filepath .Rel (resolvedBase , resolvedURL )
110136 if err != nil || strings .HasPrefix (rel , ".." ) {
111137 return fmt .Errorf ("artifact path escapes base path" )
112138 }
113- if err := os .Remove (absURL ); err != nil && ! os .IsNotExist (err ) {
139+ if err := os .Remove (resolvedURL ); err != nil && ! os .IsNotExist (err ) {
114140 return err
115141 }
116142 return nil
0 commit comments