diff --git a/CHANGELOG.md b/CHANGELOG.md index cff85944..bc41a659 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Accepted XML whitespace before exact Microsoft Project element delimiters + while preserving the linear, regex-free import scanner and rejecting + attributes, longer names, non-XML whitespace, nested unmatched blocks, and + truncated input. - Attachment-list status refresh now removes the per-row database lookup, uses a configurable bounded worker pool with per-item abortable timeouts and a request-wide latency budget, preserves stale status after downstream, diff --git a/cloud-sync.js b/cloud-sync.js index 9016cfbf..0e015ebe 100644 --- a/cloud-sync.js +++ b/cloud-sync.js @@ -741,33 +741,54 @@ function openReportModal() { export function parseMsProjectXml(xml) { // Fully linear extract (indexOf/slice) — no dynamic RegExp and no lazy // [\s\S]*? block collectors (those can quadratic-backtrack on truncated input). + const isXmlWhitespace = (charCode) => ( + charCode === 0x20 || charCode === 0x09 || charCode === 0x0d || charCode === 0x0a + ); + const findTagBoundary = (source, name, from, closing = false) => { + const prefix = `<${closing ? '/' : ''}${name}`; + let searchFrom = from; + for (;;) { + const start = source.indexOf(prefix, searchFrom); + if (start === -1) return null; + let delimiter = start + prefix.length; + while (delimiter < source.length && isXmlWhitespace(source.charCodeAt(delimiter))) { + delimiter += 1; + } + if (source.charCodeAt(delimiter) === 0x3e) { + return { start, end: delimiter + 1 }; + } + // Reject attributes, longer names, and non-XML whitespace while advancing + // past every inspected byte so malformed candidates are never rescanned. + searchFrom = Math.max(delimiter + 1, start + prefix.length); + } + }; const tag = (block, name) => { - const openingTag = `<${name}>`; - const closingTag = ``; - const valueStart = block.indexOf(openingTag); - if (valueStart === -1) return ''; - const contentStart = valueStart + openingTag.length; - const valueEnd = block.indexOf(closingTag, contentStart); - return valueEnd === -1 ? '' : block.slice(contentStart, valueEnd).trim(); + const opening = findTagBoundary(block, name, 0); + if (!opening) return ''; + const closing = findTagBoundary(block, name, opening.end, true); + const nextOpening = findTagBoundary(block, name, opening.end); + if (!closing || (nextOpening && nextOpening.start < closing.start)) return ''; + return block.slice(opening.end, closing.start).trim(); }; - const collectBlocks = (source, openTag, closeTag) => { + const collectBlocks = (source, name) => { const out = []; let from = 0; for (;;) { - const start = source.indexOf(openTag, from); - if (start === -1) break; - const contentStart = start + openTag.length; - const end = source.indexOf(closeTag, contentStart); - // Incomplete open tag: stop linearly (do not rescan the remainder). - if (end === -1) break; - out.push(source.slice(start, end + closeTag.length)); - from = end + closeTag.length; + const opening = findTagBoundary(source, name, from); + if (!opening) break; + const closing = findTagBoundary(source, name, opening.end, true); + const nextOpening = findTagBoundary(source, name, opening.end); + // Incomplete or nested same-name block: stop at the first unmatched + // opening tag instead of pairing it with a later block's closing tag. + if (!closing || (nextOpening && nextOpening.start < closing.start)) break; + out.push(source.slice(opening.start, closing.end)); + from = closing.end; } return out; }; const predecessorIds = (block) => { const ids = []; - for (const link of collectBlocks(block, '', '')) { + for (const link of collectBlocks(block, 'PredecessorLink')) { const uid = tag(link, 'PredecessorUID'); if (/^\d+$/.test(uid)) ids.push(`msp-${uid}`); } @@ -779,7 +800,7 @@ export function parseMsProjectXml(xml) { const day = (s) => (/^\d{4}-\d{2}-\d{2}/.test(s) ? s.slice(0, 10) : ''); const tasks = []; const parents = {}; // depth -> last task id at that depth - const blocks = collectBlocks(String(xml || ''), '', ''); + const blocks = collectBlocks(String(xml || ''), 'Task'); for (const block of blocks) { const uid = tag(block, 'UID'); const name = unescape(tag(block, 'Name')); diff --git a/docs/doctoring/ms-project-xml-import-boundary.md b/docs/doctoring/ms-project-xml-import-boundary.md new file mode 100644 index 00000000..0a8fa5aa --- /dev/null +++ b/docs/doctoring/ms-project-xml-import-boundary.md @@ -0,0 +1,63 @@ +# Microsoft Project XML delimiter boundary + +## Decision + +ScopeWeave's Microsoft Project import profile accepts XML whitespace between an +exact supported element name and the closing `>` delimiter. The accepted code +points are: + +- U+0020 SPACE; +- U+0009 CHARACTER TABULATION; +- U+000D CARRIAGE RETURN; and +- U+000A LINE FEED. + +The parser deliberately does not become a general XML processor. It recognizes +only the exact `Task`, `PredecessorLink`, and scalar element names already used +by the import adapter. Attributes, namespace prefixes, longer lookalike names, +non-XML whitespace, self-closing forms, nested same-name blocks, and truncated +blocks are rejected or yield no value under this narrow profile. + +## Security and complexity boundary + +The scanner remains monotonic and regex-free. It advances through every rejected +candidate and uses bounded `indexOf()` and `slice()` operations rather than +constructing dynamic regular expressions or lazy whole-document block matches. +This preserves the existing denial-of-service boundary for malformed or +adversarial uploads. + +An unmatched outer element cannot consume a later nested element's closing tag. +If another same-name opening appears before the candidate closing tag, block +collection stops at the unmatched outer element instead of silently producing a +mis-parented task. + +## Executable evidence + +`tests/unit/msproject.test.mjs` covers: + +- space, tab, carriage-return, and line-feed delimiters; +- scalar and predecessor-link elements using each allowed delimiter; +- an actual U+000B vertical tab, which is not XML whitespace; +- attributes and longer element names; +- truncated and repeated unclosed task blocks; +- nested same-name openings before a closing element; and +- the existing valid import and predecessor contracts. + +The test is already part of the full unit and coverage command paths. No package +or lockfile change is required. + +## Compatibility and rollback + +The change broadens acceptance only for documents that are conformant with the +XML whitespace production at the delimiter positions used by this adapter. +Existing byte-exact exports retain the same task identifiers, names, dates, +parents, progress, and predecessor values. + +Rollback must revert the scanner, focused tests, security documentation, +CHANGELOG entry, and this record together. Reintroducing byte-exact delimiters +would again reject standards-compliant Microsoft Project exports that contain +formatting whitespace before `>`. + +## Reference + +World Wide Web Consortium. (2008). *Extensible Markup Language (XML) 1.0 +(Fifth Edition)*. https://www.w3.org/TR/2008/REC-xml-20081126/ diff --git a/docs/security.md b/docs/security.md index 5b21a5e6..0ceee972 100644 --- a/docs/security.md +++ b/docs/security.md @@ -19,7 +19,7 @@ Every user-controlled CSV cell is neutralized when, after optional leading white ## XML imports -Microsoft Project XML extraction uses bounded `indexOf`/`slice` loops. Dynamic regular expressions and lazy whole-document block collectors are prohibited because truncated or adversarial input can cause catastrophic backtracking. +Microsoft Project XML extraction uses bounded `indexOf`/`slice` loops. Opening and closing `Task`, `PredecessorLink`, and scalar tags accept only XML whitespace (space, tab, carriage return, or line feed) between the exact element name and `>`. Attributes, longer names, and other whitespace code points are not accepted by this deliberately narrow import profile. Dynamic regular expressions and lazy whole-document block collectors are prohibited because truncated or adversarial input can cause catastrophic backtracking. ## Release verification diff --git a/tests/unit/msproject.test.mjs b/tests/unit/msproject.test.mjs index d829a32c..284cb51d 100644 --- a/tests/unit/msproject.test.mjs +++ b/tests/unit/msproject.test.mjs @@ -72,4 +72,53 @@ assert.deepEqual( const incompleteOpens = `${'9open'.repeat(5000)}`; assert.deepEqual(parseMsProjectXml(incompleteOpens), [], 'unclosed Task blocks yield no tasks'); +assert.deepEqual( + parseMsProjectXml( + '11unclosed outer12nested', + ), + [], + 'an unmatched outer Task cannot consume a nested Task closing tag', +); + +const whitespaceTags = parseMsProjectXml(` + + 8 + Whitespace-compatible task + 1 + 2026-08-11T09:00:00 + 2026-08-12T17:00:00 + + 2 + +`); +assert.equal(whitespaceTags.length, 1, 'XML whitespace before tag delimiters is accepted'); +assert.equal(whitespaceTags[0].id, 'msp-8'); +assert.equal(whitespaceTags[0].phase, 'Whitespace-compatible task'); +assert.equal(whitespaceTags[0].plannedStartDate, '2026-08-11'); +assert.equal(whitespaceTags[0].plannedEndDate, '2026-08-12'); +assert.equal(whitespaceTags[0].predecessors, 'msp-2', 'block and scalar tags share the scanner'); + +assert.deepEqual( + parseMsProjectXml('9wrong'), + [], + 'TaskX must not match Task', +); +assert.deepEqual( + parseMsProjectXml('9wrong whitespace'), + [], + 'non-XML whitespace before a delimiter is rejected', +); +assert.deepEqual( + parseMsProjectXml('10truncated'), + [], + 'truncated whitespace-delimited Task stops safely', +); +assert.deepEqual( + parseMsProjectXml( + '13outerinner1', + ), + [], + 'a nested scalar opening cannot consume the inner closing delimiter', +); + console.log('✓ MS Project import tests passed');