Skip to content
Open
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/stale-placeholder-config-json.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@aws-blocks/core": patch
---

Never serve a stale placeholder `config.json` after a deploy. The build-time placeholder (`{"_placeholder":true}`) was uploaded with the 1-year mutable cache-control (`public, s-maxage=31536000, max-age=0, must-revalidate`), and the post-deploy CloudFront invalidation targeted `/.blocks-sandbox/*` — which never matches the real cache key, because the skew-protection viewer-request function rewrites the URI to `/builds/<buildId>/.blocks-sandbox/config.json` before the cache lookup. An edge that cached the placeholder during the deploy window could keep serving it for up to a year, making `getApiUrl()` throw and every client API call fail. The placeholder is now registered as a no-cache path (`no-cache, no-store, must-revalidate`) so edges never cache it long-term, and the config deployment now also invalidates the post-rewrite key `/builds/<buildId>/.blocks-sandbox/*`.
107 changes: 107 additions & 0 deletions packages/core/src/hosting.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1455,7 +1455,7 @@
/RouteStoreKeys/.test(id),
);
assert.ok(kvKeys, 'expected a RouteStoreKeys custom resource');
const entries = JSON.parse(kvKeys![1].Properties.Entries);

Check warning on line 1458 in packages/core/src/hosting.test.ts

View workflow job for this annotation

GitHub Actions / Build, Unit Tests, E2E Local

lint/style/noNonNullAssertion

Forbidden non-null assertion.
const meta = JSON.parse(entries.meta);
return meta.bp as string;
};
Expand Down Expand Up @@ -1507,4 +1507,111 @@
);
});
});

describe('config.json stale-placeholder guard (#173)', () => {
// Helper: pull every BucketDeployment custom resource's Properties.
const bucketDeployments = (stack: Stack) => {
const crs = Template.fromStack(stack).findResources(
'Custom::CDKBucketDeployment',
);
return Object.values(crs).map(
(cr) => (cr as { Properties: Record<string, unknown> }).Properties,
);
};

it('uploads the placeholder config.json with a no-cache directive, never the 1-year mutable cache-control', () => {
// The build-time placeholder (`{_placeholder:true}`) is written into the
// static dir. If it inherits the mutable asset tier's
// `s-maxage=31536000` and an edge caches it during the deploy window, the
// edge serves the placeholder for up to a year — breaking every client
// API call. It must instead be uploaded as a no-cache path so the edge
// never caches it long-term.
createSpaBuildOutput(tmpDir);
const app = new App();
const stack = new Stack(app, 'PlaceholderCacheStack');
new Hosting(stack, 'Hosting', { root: tmpDir, api: MOCK_API });

const deployments = bucketDeployments(stack);
const cc = (p: Record<string, unknown>) =>
(p.SystemMetadata as Record<string, string> | undefined)?.['cache-control'];
const includes = (p: Record<string, unknown>) =>
(p.Include as string[] | undefined) ?? [];
const excludes = (p: Record<string, unknown>) =>
(p.Exclude as string[] | undefined) ?? [];

// A deployment must upload `.blocks-sandbox/config.json` with the
// no-cache directive (this is the placeholder upload).
const noCacheDeploy = deployments.find(
(p) =>
includes(p).includes('.blocks-sandbox/config.json') &&
cc(p) === 'no-cache, no-store, must-revalidate',
);
assert.ok(
noCacheDeploy,
'placeholder .blocks-sandbox/config.json must be uploaded with ' +
'"no-cache, no-store, must-revalidate"',
);

// No deployment may cover the placeholder with the 1-year mutable
// cache-control: the mutable/other tier must EXCLUDE it.
const leaks = deployments.filter((p) => {
const directive = cc(p);
return (
typeof directive === 'string' &&
directive.includes('s-maxage=31536000') &&
!excludes(p).includes('.blocks-sandbox/config.json')
);
});
assert.strictEqual(
leaks.length,
0,
'no mutable-tier deployment may apply s-maxage=31536000 to ' +
'.blocks-sandbox/config.json',
);
});

it('registers the placeholder as a no-cache path even for a static-only site (no api)', () => {
// The placeholder is always written (step 5), so its no-cache
// registration must not depend on `props.api`. This guards against a
// regression that moves the registration inside an `if (props.api)`
// block, which would reopen the stale-placeholder window for
// static-only sites.
createSpaBuildOutput(tmpDir);
const app = new App();
const stack = new Stack(app, 'StaticOnlyPlaceholderStack');
new Hosting(stack, 'Hosting', { root: tmpDir });

Template.fromStack(stack).hasResourceProperties(
'Custom::CDKBucketDeployment',
Match.objectLike({
Include: Match.arrayWith(['.blocks-sandbox/config.json']),
SystemMetadata: Match.objectLike({
'cache-control': 'no-cache, no-store, must-revalidate',
}),
}),
);
});

it('invalidates the post-rewrite cache key (/builds/<id>/.blocks-sandbox/*)', () => {
// The viewer-request skew-protection function rewrites the URI to
// `/builds/<buildId>/.blocks-sandbox/config.json` BEFORE the cache
// lookup, so the real edge cache key lives under `/builds/<id>/`.
// Invalidating only `/.blocks-sandbox/*` never matches it.
createSpaBuildOutput(tmpDir);
const app = new App();
const stack = new Stack(app, 'InvalidationPathStack');
new Hosting(stack, 'Hosting', { root: tmpDir, api: MOCK_API });

// Assert via Template + Match so a CDK property rename fails loudly here
// rather than silently skipping a structural-heuristic lookup.
Template.fromStack(stack).hasResourceProperties(
'Custom::CDKBucketDeployment',
Match.objectLike({
DistributionPaths: Match.arrayWith([
Match.stringLikeRegexp('^/builds/.+/\\.blocks-sandbox/\\*$'),
]),
}),
);
});
});
});
27 changes: 26 additions & 1 deletion packages/core/src/hosting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
FrameworkType,
} from '@aws-blocks/hosting';
import { BLOCKS_RPC_PREFIX, BLOCKS_AUTH_PREFIX } from './constants.js';
import { BLOCKS_SANDBOX_DIR } from './common/constants.js';
import { registerConfig } from './cdk/config-registry.js';
import { getRegisteredRoutes } from './raw-route.js';

Expand Down Expand Up @@ -415,7 +416,7 @@
...(apiUrl ? { BLOCKS_API_URL: apiUrl } : {}),
},
});
} catch (error) {

Check warning on line 419 in packages/core/src/hosting.ts

View workflow job for this annotation

GitHub Actions / Build, Unit Tests, E2E Local

lint/correctness/noUnusedVariables

This variable error is unused.
throw new Error(
`Frontend build failed: ${props.buildCommand}\n` +
`Ensure the build command is correct and all dependencies are installed.`,
Expand Down Expand Up @@ -499,6 +500,19 @@
JSON.stringify({ _placeholder: true }),
);

// ── 5a. Mark config.json as a no-cache path ─────────────────────

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: the 8-line comment narrates the full #173 bug history that's already in the commit message. Keep the non-obvious invariant inline (config.json is mutable and must not inherit the content-hashed-asset s-maxage) and move the forensics to the commit body.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done — trimmed the step-5a comment to the non-obvious invariant (config.json is mutable and must not inherit the content-hashed-asset s-maxage). The bug forensics stay in the fix commit body. Fixed in a695afb.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: the 8-line comment narrates the full #173 bug history that's already in the commit message. Keep the non-obvious invariant inline (config.json is mutable and must not inherit the content-hashed-asset s-maxage) and move the forensics to the commit body.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: the 8-line comment narrates the full #173 bug history that's already in the commit message. Keep the non-obvious invariant inline (config.json is mutable and must not inherit the content-hashed-asset s-maxage) and move the forensics to the commit body.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: the 8-line comment narrates the full #173 bug history that's already in the commit message. Keep the non-obvious invariant inline (config.json is mutable and must not inherit the content-hashed-asset s-maxage) and move the forensics to the commit body.

// config.json is a fixed-name, mutable runtime-config file: it must
// NOT inherit the content-hashed mutable-asset cache-control
// (`s-maxage=31536000`) applied to `/assets/<hash>.js`. Registering it
// as a no-cache path uploads the build-time placeholder with
// `no-cache, no-store, must-revalidate` so an edge never caches it
// long-term; the real config is deployed in step 8 with `max-age=60`.
const configNoCachePath = `${BLOCKS_SANDBOX_DIR}/config.json`;
const existingNoCachePaths = manifest.staticAssets.noCachePaths ?? [];
manifest.staticAssets.noCachePaths = existingNoCachePaths.includes(configNoCachePath)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The noCachePaths registration runs unconditionally (no api guard), which is correct — but both new tests always pass MOCK_API. A static-only test (no api prop) would guard against a future regression where the registration is accidentally moved inside the if (props.api) block, since the placeholder is always written.

@fossamagna fossamagna Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added a static-only test (no api prop) — registers the placeholder as a no-cache path even for a static-only site (no api) — asserting the placeholder is uploaded with no-cache, no-store, must-revalidate. Since the placeholder is always written, this guards against a regression that moves the noCachePaths registration inside an if (props.api) block. Fixed in a695afb.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The noCachePaths registration runs unconditionally (no api guard), which is correct — but both new tests always pass MOCK_API. A static-only test (no api prop) would guard against a future regression where the registration is accidentally moved inside the if (props.api) block, since the placeholder is always written.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The noCachePaths registration runs unconditionally (no api guard), which is correct — but both new tests always pass MOCK_API. A static-only test (no api prop) would guard against a future regression where the registration is accidentally moved inside the if (props.api) block, since the placeholder is always written.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The noCachePaths registration runs unconditionally (no api guard), which is correct — but both new tests always pass MOCK_API. A static-only test (no api prop) would guard against a future regression where the registration is accidentally moved inside the if (props.api) block, since the placeholder is always written.

? existingNoCachePaths
: [...existingNoCachePaths, configNoCachePath];

// ── 5b. Inject static route for .blocks-sandbox config ──────────
// Insert a static route for /.blocks-sandbox/* so CloudFront
// serves config.json from S3 instead of routing to compute.
Expand Down Expand Up @@ -596,7 +610,18 @@
destinationKeyPrefix: `builds/${buildId}/.blocks-sandbox`,
prune: false,
distribution: hosting.distribution,
distributionPaths: ['/.blocks-sandbox/*'],
// The skew-protection viewer-request CloudFront function rewrites the
// URI to `/builds/<buildId>/.blocks-sandbox/config.json` BEFORE the
// cache lookup, so the real edge cache key lives under `/builds/<id>/`.
// Invalidating only `/.blocks-sandbox/*` never matches that key and is
// a no-op for config.json. Invalidate the post-rewrite key too. (The
// primary guard against staleness is step 5a's no-cache placeholder;
// this is defense-in-depth so a post-deploy invalidation actually
// clears any edge entry at its real key.)
distributionPaths: [
`/builds/${buildId}/.blocks-sandbox/*`,
'/.blocks-sandbox/*',
],
cacheControl: [s3deploy.CacheControl.fromString('public, max-age=60, must-revalidate')],
});

Expand Down Expand Up @@ -718,7 +743,7 @@
let behaviorPattern: string;
const paramIndex = route.path.indexOf('/{');
if (paramIndex !== -1) {
behaviorPattern = route.path.substring(0, paramIndex) + '/*';

Check notice on line 746 in packages/core/src/hosting.ts

View workflow job for this annotation

GitHub Actions / Build, Unit Tests, E2E Local

lint/style/useTemplate

Template literals are preferred over string concatenation.
} else {
behaviorPattern = route.path;
}
Expand Down
Loading