Skip to content

Dev #1153

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.

Sign up for GitHub

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

Merged
vikrantraut-cstk merged 8 commits into from
Aug 26, 2026
Merged

Dev #1153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e3d9df5
fix: stop excluding AEM structure entry from migration (CMG-1112)
ShradhaNahar Aug 26, 2026
cc632b4
fix: stop excluding AEM structure entry from migration (CMG-1112)
ShradhaNahar Aug 26, 2026
cf6f1c3
added back correct file parth
ShradhaNahar Aug 26, 2026
0d8faf4
config fixes
ShradhaNahar Aug 26, 2026
e002672
fix: bump react-router and postcss to close SLA-breached CVEs
ShradhaNahar Aug 26, 2026
9708bdb
fix: reserve AEM entry collision key only once an entry is emitted
ShradhaNahar Aug 26, 2026
014cfa9
chore: drop incidental upload-api/src/config formatting-only changes
ShradhaNahar Aug 26, 2026
c2d98f2
Merge pull request #1152 from contentstack/bugfix/aem-cmg-1112
vikrantraut-cstk Aug 26, 2026
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
35 changes: 13 additions & 22 deletions api/src/services/aem.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1363,8 +1363,11 @@ const createEntry = async ({
const entryMapping: Record<string, string[]> = {};
const usedEntryUids = new Set<string>();

// Process each entry file
for await (const fileName of read(entriesDir)) {
// Process each entry file. Sorted (not raw fs-readdir-recursive order) so that when two
// files legitimately collide on the same modelId+locale (see CMG-1112), which one "wins"
// and gets migrated is deterministic and reproducible across machines/runs, rather than
// depending on filesystem directory order.
for await (const fileName of [...read(entriesDir)].sort()) {

@umesh-more-cstk umesh-more-cstk Aug 26, 2026

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.

question: Sorting makes the collision winner deterministic, but it doesn't make it correct — and the at most one entry per modelId::locale invariant is unchanged by this PR. So when the template structure export and a real page derive the same modelId (the exact CMG-1112 case the deleted block described), exactly one of them can still be migrated, and which one is now decided purely by lexicographic path order.

Failure scenario, if the structure export does resolve a content type (i.e. it carries a templateName/templateType that matches an otherCmsUid, or an isXF title match): conf/... sorts before content/... ("conf" < "cont"), so the structure file is processed first, matches at line 1432, reserves modelId::locale at 1437, and the real page then hits the duplicate check at ~1408 and is dropped with a Skipped duplicate entry warning. That swaps the reported symptom (structure entry missing) for a worse one (the actual page missing), and it's silent apart from a warn log.

Conversely, if the structure export never matches a content type — which is what 9708bdb's message assumes — then it can never emit an entry at all, and deleting the exclusion block is a no-op for output: the deferred reservation alone fixes the reopened bug. Either way the exclusion removal doesn't get the structure entry migrated.

Could you confirm which case the CMG-1112 export actually hits? If it's the first, the fix wants to be at the uid derivation rather than the walk order — fold the disambiguating path (dataLayer[id]['repo:path'] or :path) into modelId when two files share parseData.id, so both can be emitted instead of one shadowing the other; or, if only one should ever win, encode that precedence explicitly (real content beats template structure) rather than leaving it to sort().

Generated by Claude Code

@umesh-more-cstk umesh-more-cstk Aug 26, 2026

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.

question: The sibling layer wasn't sorted with it. upload-api/migration-aem/libs/entries/index.ts:58 still walks read(templatesDir) in raw fs order, and both files carry comments asserting the two collision policies must stay in lockstep ("the way extractEntries's own collision policy already works" here; "must match createEntry in the api's aem.service" there).

Failure scenario: for a modelId that two files collide on, extractEntries keeps the fs-order-first file and createEntry now keeps the sort-order-first file. When those differ, the entry-mapper row (entryName, language, otherCmsCTName — index.ts:102-110) describes the losing file while the entry actually written to Contentstack came from the winner. The uid matches, so nothing breaks loudly; the Entry Mapper UI just shows the wrong source name/locale for that row.

Suggested fix: apply the same [...read(templatesDir)].sort() at index.ts:58 so both layers pick the same winner.

Unrelated to this PR, but noting it while we're here: extractEntries keys on modelId alone (index.ts:97) whereas createEntry keys on modelId::mappedLocale. For a page with several locale variants, createEntry correctly emits one entry per locale, but only the first locale gets a mapper row — worth a follow-up ticket if that's not already known.

Generated by Claude Code

const filePath = path.join(entriesDir, fileName);
if (filePath?.startsWith?.(damPath)) {
continue;
Expand All @@ -1373,23 +1376,6 @@ const createEntry = async ({
if (typeof content === 'string') {
const parseData = JSON.parse(content);

// AEM can export a page TEMPLATE's own structure/schema definition (e.g.
// /conf/.../settings/wcm/templates/<template>/structure[.html]) as a separate file
// alongside real pages that use that template — it isn't content, just the
// template's own component-allow-list. Exclude it outright rather than letting it
// compete with a real page for the same derived id (see CMG-1112): this removes the
// ambiguity entirely instead of leaving the outcome dependent on directory-walk order.
const repoPath: string | undefined = parseData?.dataLayer?.[parseData?.id]?.['repo:path'];
if (repoPath && /\/settings\/wcm\/templates\/[^/]+\/structure(\.html)?$/.test(repoPath)) {
await customLogger(
projectId,
destinationStackId,
'warn',
getLogMessage(srcFunc, `Skipped entry from "${fileName}": AEM template structure/schema definition, not real content (repo:path "${repoPath}").`, {})
);
continue;
}

// Use the page model's stable "id" as the entry uid so uid-mapper keys
// stay consistent across delta iterations; random uuid only as fallback.
let modelId = typeof parseData?.id === 'string' && parseData.id.trim() !== ''
Expand Down Expand Up @@ -1430,9 +1416,6 @@ const createEntry = async ({
continue;
}
const uid = modelId || uuidv4?.()?.replace?.(/-/g, '');
if (collisionKey) {
usedEntryUids.add(collisionKey);
}
const title = getTitle(parseData);
const isEFragment = isExperienceFragment(parseData);
const templateUid = isEFragment?.isXF ? parseData?.title : parseData?.templateName ?? parseData?.templateType;
Expand All @@ -1446,6 +1429,14 @@ const createEntry = async ({
data.publish_details = [];

if (contentType?.contentstackUid && data && mappedLocale) {
// Reserve the collision key only now that an entry is actually being emitted — a
// file that reaches the "no content type matched" / "no mapped locale" branch below
// must NOT consume the key, or it would permanently block a sibling file (sharing
// the same modelId::locale) that could otherwise have produced the real entry,
// leaving zero entries instead of one.
if (collisionKey) {
usedEntryUids.add(collisionKey);
}
const mappedValue = (keyMapper as Record<string, string> | undefined)?.[contentType.contentstackUid];
const resolvedCtUid: string =
mappedValue && mappedValue !== ''
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"overrides": {
"axios": ">=1.16.0",
"nth-check": ">=2.0.1",
"postcss": ">=8.5.10",
"postcss": ">=8.5.23",

@umesh-more-cstk umesh-more-cstk Aug 26, 2026

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.

blocker: This override doesn't reach anything, and two of the three workspaces stay on the vulnerable postcss.

Verified against the lockfiles at this head:

lockfile postcss nanoid
package-lock.json (root) not present not present
api/package-lock.json 8.5.15 3.3.12
upload-api/package-lock.json 8.5.20 3.3.16
ui/package-lock.json 8.5.26 ✅ 3.3.18 ✅

Root package.json has no workspaces key, and api/, upload-api/, ui/ each have their own package.json + lockfile and are installed independently — so a root overrides entry propagates to none of them. Root's own tree doesn't contain postcss either, so this line is inert on both counts; only the matching entry added to ui/package.json did any work.

Failure scenario: the CVE this PR is titled for (e002672, "close SLA-breached CVEs") stays open in api and upload-api at 8.5.15/8.5.20 — both below the >=8.5.23 floor this PR itself declares — while the scanner shows the root and ui manifests as remediated. The SLA ticket gets closed on a partial fix.

Suggested fix: add "postcss": ">=8.5.23" to the overrides block of api/package.json and upload-api/package.json, then regenerate api/package-lock.json and upload-api/package-lock.json (npm install --package-lock-only in each). That pulls nanoid up as a postcss dependency at the same time. Keep or drop this root line as you prefer — it has no effect either way.

Generated by Claude Code

"serialize-javascript": ">=6.0.2",
"@babel/runtime": ">=7.26.10",
"lodash": "^4.18.1",
Expand Down
33 changes: 17 additions & 16 deletions ui/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions ui/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
"react-dom": "^18.2.0",
"react-final-form": "^6.5.9",
"react-redux": "^9.1.2",
"react-router": "^7.15.0",
"react-router-dom": "^7.15.0",
"react-router": "^7.18.2",
"react-router-dom": "^7.18.2",
"react-toastify": "npm:@contentstack/react-toastify@6.1.5",
"redux-persist": "^6.0.0",
"sass": "^1.68.0",
Expand Down Expand Up @@ -71,7 +71,8 @@
"ws": ">=8.21.0",
"esbuild": "^0.28.1",
"fast-uri": ">=4.1.1",
"brace-expansion": ">=5.0.9"
"brace-expansion": ">=5.0.9",
"postcss": ">=8.5.23"
},
"eslintConfig": {
"extends": [
Expand Down
Loading