CR updates, update CLI to be more forceful in rejecting weird relative urls
Signed-off-by: Aramis Sennyey <sennyeya@amazon.com>
This commit is contained in:
@@ -2,4 +2,4 @@
|
||||
'@backstage/core-app-api': patch
|
||||
---
|
||||
|
||||
Updated AppManager to return an absolute URL for `backend.baseUrl` when a relative URL is provided.
|
||||
Apps will now detect when a relative `backend.baseUrl` is provided and update the config accordingly.
|
||||
|
||||
@@ -64,11 +64,10 @@ export const apis = [
|
||||
createApiFactory({
|
||||
api: discoveryApiRef,
|
||||
deps: { configApi: configApiRef },
|
||||
factory: ({ configApi }) => {
|
||||
return UrlPatternDiscovery.compile(
|
||||
factory: ({ configApi }) =>
|
||||
UrlPatternDiscovery.compile(
|
||||
`${configApi.getString('backend.baseUrl')}/api/{{ pluginId }}`,
|
||||
);
|
||||
},
|
||||
),
|
||||
}),
|
||||
createApiFactory({
|
||||
api: alertApiRef,
|
||||
|
||||
@@ -39,14 +39,6 @@ export function registerRepoCommand(program: Command) {
|
||||
'--all',
|
||||
'Build all packages, including bundled app and backend packages.',
|
||||
)
|
||||
.option(
|
||||
'--public-path <path>',
|
||||
'Public path for hosting the website on, can be relative.',
|
||||
)
|
||||
.option(
|
||||
'--backend-url <url>',
|
||||
'Backend url, expects just the origin or sub-route. Do not include /api. Can be relative.',
|
||||
)
|
||||
.option(
|
||||
'--since <ref>',
|
||||
'Only build packages and their dev dependents that changed since the specified ref',
|
||||
|
||||
@@ -32,13 +32,13 @@ describe('readCliConfig', () => {
|
||||
it('should return backend.baseUrl when backendUrl present in cli options', () => {
|
||||
expect(
|
||||
readCliConfig({
|
||||
backendUrl: 'http://localhost:3000',
|
||||
backendUrl: '/',
|
||||
}),
|
||||
).toEqual([
|
||||
{
|
||||
data: {
|
||||
backend: {
|
||||
baseUrl: 'http://localhost:3000',
|
||||
baseUrl: '/',
|
||||
},
|
||||
},
|
||||
context: 'cli',
|
||||
@@ -49,13 +49,13 @@ describe('readCliConfig', () => {
|
||||
it('should return app.baseUrl when publicPath present in cli options', () => {
|
||||
expect(
|
||||
readCliConfig({
|
||||
publicPath: 'http://localhost:3000',
|
||||
publicPath: '/',
|
||||
}),
|
||||
).toEqual([
|
||||
{
|
||||
data: {
|
||||
app: {
|
||||
baseUrl: 'http://localhost:3000',
|
||||
baseUrl: '/',
|
||||
},
|
||||
},
|
||||
context: 'cli',
|
||||
@@ -66,21 +66,41 @@ describe('readCliConfig', () => {
|
||||
it('should return app.baseUrl and backend.baseUrl when publicPath and backendUrl present in cli options', () => {
|
||||
expect(
|
||||
readCliConfig({
|
||||
publicPath: 'http://localhost:3000',
|
||||
backendUrl: 'http://localhost:3000/api',
|
||||
publicPath: '/',
|
||||
backendUrl: '/api',
|
||||
}),
|
||||
).toEqual([
|
||||
{
|
||||
data: {
|
||||
app: {
|
||||
baseUrl: 'http://localhost:3000',
|
||||
baseUrl: '/',
|
||||
},
|
||||
backend: {
|
||||
baseUrl: 'http://localhost:3000/api',
|
||||
baseUrl: '/api',
|
||||
},
|
||||
},
|
||||
context: 'cli',
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it('should throw for public paths that do NOT start with /', () => {
|
||||
['http://localhost:3000', './', '../..'].forEach(publicPath =>
|
||||
expect(() => {
|
||||
readCliConfig({
|
||||
publicPath,
|
||||
});
|
||||
}).toThrow('Public path must be relative'),
|
||||
);
|
||||
});
|
||||
|
||||
it('should throw for backend urls that do NOT start with /', () => {
|
||||
['http://localhost:3000', './', '../..'].forEach(backendUrl =>
|
||||
expect(() => {
|
||||
readCliConfig({
|
||||
backendUrl,
|
||||
});
|
||||
}).toThrow('Backend URL must be relative'),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -52,16 +52,23 @@ export function readCliConfig(opts?: CliConfigOptions): AppConfig[] {
|
||||
if (!opts || Object.keys(opts).length === 0) return [];
|
||||
const data: JsonObject = {};
|
||||
|
||||
if (opts.publicPath) {
|
||||
if (opts.publicPath?.startsWith('/')) {
|
||||
data.app = {
|
||||
baseUrl: opts.publicPath,
|
||||
};
|
||||
} else if (opts.publicPath) {
|
||||
throw new Error(
|
||||
'Public path must be relative and start with "/" when specified through CLI options. Path traversals like "./" and assumed relative endpoints like "backstage" are not supported.',
|
||||
);
|
||||
}
|
||||
|
||||
if (opts.backendUrl) {
|
||||
if (opts.backendUrl?.startsWith('/')) {
|
||||
data.backend = {
|
||||
baseUrl: opts.backendUrl,
|
||||
};
|
||||
} else if (opts.backendUrl) {
|
||||
throw new Error(
|
||||
'Backend URL must be relative and start with "/" when specified through CLI options. Path traversals like "./" and assumed relative endpoint like "api" are not supported.',
|
||||
);
|
||||
}
|
||||
|
||||
if (Object.keys(data).length === 0) return [];
|
||||
|
||||
@@ -179,9 +179,12 @@ function useConfigLoader(
|
||||
context: 'relative-override',
|
||||
};
|
||||
|
||||
configReader = ConfigReader.fromConfigs(
|
||||
config.value ? [...config.value, relativeBackendConfig] : [],
|
||||
);
|
||||
// Config reader may not have backend.baseUrl set. config.value may be undefined.
|
||||
if (configReader.getOptionalString('backend.baseUrl')?.startsWith('/')) {
|
||||
config.value?.push(relativeBackendConfig);
|
||||
}
|
||||
|
||||
configReader = ConfigReader.fromConfigs(config.value ?? []);
|
||||
|
||||
return { api: configReader };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user