Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/copy-move-set-object-access.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@tigrisdata/storage': minor
---

Add `copy(src, dest, options?)` and `move(src, dest, options?)` for copying and moving objects within or across buckets. Add `setObjectAccess(path, { access })` for changing object ACLs. Deprecate `updateObject`; use `setObjectAccess` for ACL changes and `move` for renames.
5 changes: 5 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

## Code Quality & Security

### Agent Behavior

- **Never `git commit`, `git push`, or open a PR without an explicit, in-the-moment user instruction.** Treat each commit, push, and PR as a separate confirmation: approval for one does not extend to the next. When unsure, stop and ask.
- Stage proposed changes and surface them in chat first; let the user decide when (and whether) to commit. The same rule applies to fixup commits, follow-up commits, and changeset commits — none are implied by an earlier "commit" instruction.

### Commit Guidelines

Commit messages follow **Conventional Commits** format:
Expand Down
2 changes: 1 addition & 1 deletion biome.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"$schema": "https://biomejs.dev/schemas/2.4.13/schema.json",
"$schema": "https://biomejs.dev/schemas/2.4.14/schema.json",
"vcs": {
"enabled": true,
"clientKind": "git",
Expand Down
86 changes: 86 additions & 0 deletions packages/storage/src/lib/object/copy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import { TigrisHeaders } from '@shared/headers';
import { handleError } from '@shared/utils';
import { config, missingConfigError } from '../config';
import { createStorageClient } from '../http-client';
import type { TigrisStorageConfig, TigrisStorageResponse } from '../types';

export type CopyOptions = {
config?: TigrisStorageConfig;
/** Source bucket. Defaults to `config.bucket`. */
srcBucket?: string;
/** Destination bucket. Defaults to `srcBucket` (same-bucket copy). */
destBucket?: string;
};

export type CopyResponse = {
src: string;
dest: string;
};

export async function copy(
src: string,
dest: string,
options?: CopyOptions
): Promise<TigrisStorageResponse<CopyResponse, Error>> {
return copyOrMove(src, dest, false, options);
}

export async function copyOrMove(
Comment thread
designcode marked this conversation as resolved.
src: string,
dest: string,
rename: boolean,
options?: CopyOptions
): Promise<TigrisStorageResponse<CopyResponse, Error>> {
if (!src || !dest) {
return { error: new Error('src and dest are required') };
}

const srcBucket =
options?.srcBucket ?? options?.config?.bucket ?? config.bucket;

if (!srcBucket) {
return missingConfigError('bucket');
}

const destBucket = options?.destBucket ?? srcBucket;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Empty destBucket string bypasses bucket validation

Low Severity

The srcBucket value is validated for emptiness with if (!srcBucket), but destBucket has no equivalent check. If a caller explicitly passes destBucket: '', the nullish coalescing operator (??) won't fall through to srcBucket because empty string isn't nullish. This results in a malformed request path like //${encodeURIComponent(dest)}?x-id=CopyObject instead of a clear validation error.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d2e363a. Configure here.


if (srcBucket === destBucket && src === dest) {
return { error: new Error('src and dest must differ') };
}

const { data: storageHttpClient, error: storageHttpClientError } =
createStorageClient(options?.config);

if (storageHttpClientError) {
return { error: storageHttpClientError };
}

const headers: Record<string, string> = {
[TigrisHeaders.COPY_SOURCE]: `${srcBucket}/${encodeURIComponent(src)}`,
};

if (rename) {
headers[TigrisHeaders.RENAME] = 'true';
}

try {
const response = await storageHttpClient.request({
method: 'PUT',
path: `/${destBucket}/${encodeURIComponent(dest)}?x-id=CopyObject`,
headers,
});

if (response.error) {
return { error: response.error };
}
} catch (error) {
return handleError(error as Error);
}

return {
data: {
src: `${srcBucket}/${src}`,
dest: `${destBucket}/${dest}`,
},
};
}
13 changes: 13 additions & 0 deletions packages/storage/src/lib/object/move.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import type { TigrisStorageResponse } from '../types';
import { type CopyOptions, type CopyResponse, copyOrMove } from './copy';

export type MoveOptions = CopyOptions;
export type MoveResponse = CopyResponse;

export async function move(
src: string,
dest: string,
options?: MoveOptions
): Promise<TigrisStorageResponse<MoveResponse, Error>> {
return copyOrMove(src, dest, true, options);
}
53 changes: 53 additions & 0 deletions packages/storage/src/lib/object/set/access.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { PutObjectAclCommand } from '@aws-sdk/client-s3';
import { handleError } from '@shared/utils';
import { config, missingConfigError } from '../../config';
import { createTigrisClient } from '../../tigris-client';
import type { TigrisStorageConfig, TigrisStorageResponse } from '../../types';

export type SetObjectAccessOptions = {
config?: TigrisStorageConfig;
access: 'public' | 'private';
};

export type SetObjectAccessResponse = {
path: string;
};

export async function setObjectAccess(
path: string,
options: SetObjectAccessOptions
): Promise<TigrisStorageResponse<SetObjectAccessResponse, Error>> {
if (!options?.access) {
return { error: new Error('No access option provided') };
}
Comment thread
designcode marked this conversation as resolved.

const bucket = options?.config?.bucket ?? config.bucket;

if (!bucket) {
return missingConfigError('bucket');
}

const { data: tigrisClient, error } = createTigrisClient(options?.config);

if (error) {
return { error };
}

try {
await tigrisClient.send(
new PutObjectAclCommand({
Bucket: bucket,
Key: path,
ACL: options.access === 'public' ? 'public-read' : 'private',
})
);
} catch (error) {
return handleError(error as Error);
}

return {
data: {
path,
},
};
}
5 changes: 5 additions & 0 deletions packages/storage/src/lib/object/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ export type UpdateObjectResponse = {
path: string;
};

/**
* @deprecated Use `setObjectAccess` to change object ACLs, or the dedicated
* rename helper to change an object's key. `updateObject` will be removed in
* a future major version.
*/
Comment thread
designcode marked this conversation as resolved.
export async function updateObject(
path: string,
options?: UpdateObjectOptions
Expand Down
7 changes: 7 additions & 0 deletions packages/storage/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export {
type BundleResponse,
bundle,
} from './lib/object/bundle';
export { type CopyOptions, type CopyResponse, copy } from './lib/object/copy';
export { type GetOptions, type GetResponse, get } from './lib/object/get';
export { type HeadOptions, type HeadResponse, head } from './lib/object/head';
export {
Expand All @@ -73,6 +74,7 @@ export {
list,
} from './lib/object/list';
export { isMigrated, type MigrateOptions, migrate } from './lib/object/migrate';
export { type MoveOptions, type MoveResponse, move } from './lib/object/move';
export {
type CompleteMultipartUploadOptions,
type CompleteMultipartUploadResponse,
Expand All @@ -91,6 +93,11 @@ export {
} from './lib/object/presigned-url';
export { type PutOptions, type PutResponse, put } from './lib/object/put';
export { type RemoveOptions, remove } from './lib/object/remove';
export {
type SetObjectAccessOptions,
type SetObjectAccessResponse,
setObjectAccess,
} from './lib/object/set/access';
export {
type UpdateObjectOptions,
type UpdateObjectResponse,
Expand Down
129 changes: 85 additions & 44 deletions packages/storage/src/test/integration.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import { config } from '../lib/config';
import { copy } from '../lib/object/copy';
import { get } from '../lib/object/get';
import { head } from '../lib/object/head';
import { list } from '../lib/object/list';
import { move } from '../lib/object/move';
import { put } from '../lib/object/put';
import { remove } from '../lib/object/remove';
import { updateObject } from '../lib/object/update';
import { setObjectAccess } from '../lib/object/set/access';
import { shouldSkipIntegrationTests } from './setup';

const skipTests = shouldSkipIntegrationTests();
Expand Down Expand Up @@ -242,76 +244,115 @@ describe.skipIf(skipTests)('Tigris Storage Integration Tests', () => {
});
});

describe('updateObject', () => {
const updateFileName = `test-update-${Date.now()}.txt`;
describe('setObjectAccess', () => {
const accessFileName = `test-access-${Date.now()}.txt`;

beforeEach(async () => {
await put(updateFileName, 'update test content', { config });
await put(accessFileName, 'access test content', { config });
});

afterEach(async () => {
// Clean up both old and potentially renamed files
await remove(updateFileName, { config });
await remove(accessFileName, { config });
});

it('should return error when no options provided', async () => {
const result = await updateObject(updateFileName);
expect(result.error).toBeDefined();
expect(result.error?.message).toBe('No update options provided');
});

it('should rename an object', async () => {
const newKey = `test-renamed-${Date.now()}.txt`;
const result = await updateObject(updateFileName, {
key: newKey,
it('should set access to public', async () => {
const result = await setObjectAccess(accessFileName, {
access: 'public',
config,
});

expect(result.error).toBeUndefined();
expect(result.data?.path).toBe(newKey);

// Rename back so subsequent tests still find the original file
await updateObject(newKey, { key: updateFileName, config });
expect(result.data?.path).toBe(accessFileName);
});

it('should update access to public', async () => {
const result = await updateObject(updateFileName, {
access: 'public',
it('should set access to private', async () => {
const result = await setObjectAccess(accessFileName, {
access: 'private',
config,
});

expect(result.error).toBeUndefined();
expect(result.data?.path).toBe(updateFileName);
expect(result.data?.path).toBe(accessFileName);
});
});

it('should update access to private', async () => {
const result = await updateObject(updateFileName, {
access: 'private',
config,
});
describe('copy', () => {
const srcFileName = `test-copy-src-${Date.now()}.txt`;
const srcContent = 'copy test content';
const createdKeys: string[] = [];

beforeEach(async () => {
await put(srcFileName, srcContent, { config });
});

afterEach(async () => {
await remove(srcFileName, { config });
await Promise.all(createdKeys.map((key) => remove(key, { config })));
createdKeys.length = 0;
});

it('should return error when src and dest are identical', async () => {
const result = await copy(srcFileName, srcFileName, { config });
expect(result.error?.message).toBe('src and dest must differ');
});

it('should copy an object and leave the source intact', async () => {
const destKey = `test-copy-dest-${Date.now()}.txt`;
createdKeys.push(destKey);

const result = await copy(srcFileName, destKey, { config });

expect(result.error).toBeUndefined();
expect(result.data?.path).toBe(updateFileName);
expect(result.data?.src).toBe(`${config.bucket}/${srcFileName}`);
expect(result.data?.dest).toBe(`${config.bucket}/${destKey}`);

// Source still exists.
const srcHead = await head(srcFileName, { config });
expect(srcHead.data).toBeDefined();

// Destination has the same content.
const destGet = await get(destKey, 'string', { config });
expect(destGet.data).toBe(srcContent);
});
});

it('should rename and update access together', async () => {
const newKey = `test-renamed-public-${Date.now()}.txt`;
describe('move', () => {
const srcFileName = `test-move-src-${Date.now()}.txt`;
const srcContent = 'move test content';
const createdKeys: string[] = [];

const result = await updateObject(updateFileName, {
key: newKey,
access: 'public',
config,
});
beforeEach(async () => {
await put(srcFileName, srcContent, { config });
});

afterEach(async () => {
await remove(srcFileName, { config });
await Promise.all(createdKeys.map((key) => remove(key, { config })));
createdKeys.length = 0;
});

it('should return error when src and dest are identical', async () => {
const result = await move(srcFileName, srcFileName, { config });
expect(result.error?.message).toBe('src and dest must differ');
});

it('should move an object and remove the source', async () => {
const destKey = `test-move-dest-${Date.now()}.txt`;
createdKeys.push(destKey);

const result = await move(srcFileName, destKey, { config });

expect(result.error).toBeUndefined();
expect(result.data?.path).toBe(newKey);
expect(result.data?.src).toBe(`${config.bucket}/${srcFileName}`);
expect(result.data?.dest).toBe(`${config.bucket}/${destKey}`);

// Rename back so subsequent tests still find the original file
await updateObject(newKey, {
key: updateFileName,
access: 'private',
config,
});
// Source no longer exists.
const srcHead = await head(srcFileName, { config });
expect(srcHead.data).toBeUndefined();

// Destination has the original content.
const destGet = await get(destKey, 'string', { config });
expect(destGet.data).toBe(srcContent);
});
});

Expand Down
Loading