-
Notifications
You must be signed in to change notification settings - Fork 228
Asset pipeline flow for hosted static app #6806
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| import spec from './app_config_hosted_app_home.js' | ||
| import {placeholderAppConfiguration} from '../../app/app.test-data.js' | ||
| import {copyDirectoryContents} from '@shopify/cli-kit/node/fs' | ||
| import {describe, expect, test, vi} from 'vitest' | ||
|
|
||
| vi.mock('@shopify/cli-kit/node/fs') | ||
|
|
||
| describe('hosted_app_home', () => { | ||
| describe('transform', () => { | ||
| test('should return the transformed object with static_root', () => { | ||
| const object = { | ||
| static_root: 'public', | ||
| } | ||
| const appConfigSpec = spec | ||
|
|
||
| const result = appConfigSpec.transformLocalToRemote!(object, placeholderAppConfiguration) | ||
|
|
||
| expect(result).toMatchObject({ | ||
| static_root: 'public', | ||
| }) | ||
| }) | ||
|
|
||
| test('should return empty object when static_root is not provided', () => { | ||
| const object = {} | ||
| const appConfigSpec = spec | ||
|
|
||
| const result = appConfigSpec.transformLocalToRemote!(object, placeholderAppConfiguration) | ||
|
|
||
| expect(result).toMatchObject({}) | ||
| }) | ||
| }) | ||
|
|
||
| describe('reverseTransform', () => { | ||
| test('should return the reversed transformed object with static_root', () => { | ||
| const object = { | ||
| static_root: 'public', | ||
| } | ||
| const appConfigSpec = spec | ||
|
|
||
| const result = appConfigSpec.transformRemoteToLocal!(object) | ||
|
|
||
| expect(result).toMatchObject({ | ||
| static_root: 'public', | ||
| }) | ||
| }) | ||
|
|
||
| test('should return empty object when static_root is not provided', () => { | ||
| const object = {} | ||
| const appConfigSpec = spec | ||
|
|
||
| const result = appConfigSpec.transformRemoteToLocal!(object) | ||
|
|
||
| expect(result).toMatchObject({}) | ||
| }) | ||
| }) | ||
|
|
||
| describe('copyStaticAssets', () => { | ||
| test('should copy static assets from source to output directory', async () => { | ||
| vi.mocked(copyDirectoryContents).mockResolvedValue(undefined) | ||
| const config = {static_root: 'public'} | ||
| const directory = '/app/root' | ||
| const outputPath = '/output/dist/bundle.js' | ||
|
|
||
| await spec.copyStaticAssets!(config, directory, outputPath) | ||
|
|
||
| expect(copyDirectoryContents).toHaveBeenCalledWith('/app/root/public', '/output/dist') | ||
| }) | ||
|
|
||
| test('should not copy assets when static_root is not provided', async () => { | ||
| const config = {} | ||
| const directory = '/app/root' | ||
| const outputPath = '/output/dist/bundle.js' | ||
|
|
||
| await spec.copyStaticAssets!(config, directory, outputPath) | ||
|
|
||
| expect(copyDirectoryContents).not.toHaveBeenCalled() | ||
| }) | ||
|
|
||
| test('should throw error when copy fails', async () => { | ||
| vi.mocked(copyDirectoryContents).mockRejectedValue(new Error('Permission denied')) | ||
| const config = {static_root: 'public'} | ||
| const directory = '/app/root' | ||
| const outputPath = '/output/dist/bundle.js' | ||
|
|
||
| await expect(spec.copyStaticAssets!(config, directory, outputPath)).rejects.toThrow( | ||
| 'Failed to copy static assets from /app/root/public to /output/dist: Permission denied', | ||
| ) | ||
| }) | ||
| }) | ||
|
|
||
| describe('buildConfig', () => { | ||
| test('should have hosted_app_home build mode', () => { | ||
| expect(spec.buildConfig).toEqual({mode: 'hosted_app_home'}) | ||
| }) | ||
| }) | ||
|
|
||
| describe('identifier', () => { | ||
| test('should have correct identifier', () => { | ||
| expect(spec.identifier).toBe('hosted_app_home') | ||
| }) | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| import {BaseSchemaWithoutHandle} from '../schemas.js' | ||
| import {TransformationConfig, createConfigExtensionSpecification} from '../specification.js' | ||
| import {copyDirectoryContents} from '@shopify/cli-kit/node/fs' | ||
| import {dirname, joinPath} from '@shopify/cli-kit/node/path' | ||
| import {zod} from '@shopify/cli-kit/node/schema' | ||
|
|
||
| const HostedAppHomeSchema = BaseSchemaWithoutHandle.extend({ | ||
| static_root: zod.string().optional(), | ||
| }) | ||
|
|
||
| const HostedAppHomeTransformConfig: TransformationConfig = { | ||
| static_root: 'static_root', | ||
| } | ||
|
|
||
| export const HostedAppHomeSpecIdentifier = 'hosted_app_home' | ||
|
|
||
| const hostedAppHomeSpec = createConfigExtensionSpecification({ | ||
| identifier: HostedAppHomeSpecIdentifier, | ||
| buildConfig: {mode: 'hosted_app_home'} as const, | ||
| schema: HostedAppHomeSchema, | ||
| transformConfig: HostedAppHomeTransformConfig, | ||
| copyStaticAssets: async (config, directory, outputPath) => { | ||
| if (!config.static_root) return | ||
| const sourceDir = joinPath(directory, config.static_root) | ||
| const outputDir = dirname(outputPath) | ||
|
|
||
| return copyDirectoryContents(sourceDir, outputDir).catch((error) => { | ||
| throw new Error(`Failed to copy static assets from ${sourceDir} to ${outputDir}: ${error.message}`) | ||
| }) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Catch handler assumes The catch handler assumes Evidence: .catch((error) => {
throw new Error(`...: ${error.message}`)
})Impact:
|
||
| }, | ||
| }) | ||
|
|
||
| export default hostedAppHomeSpec | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validate
static_rootto prevent path traversal/arbitrary file copyingstatic_rootis taken directly from config and joined intosourceDir(joinPath(directory, config.static_root)). If misconfigured or attacker-controlled, values like../.., absolute paths, or symlinks can escape the app directory and copy arbitrary filesystem contents into the deploy bundle. This is risky even if developer-controlled (CI agents often have adjacent secrets). Recursive copy may amplify impact, especially if symlinks are followed.