Confluence page import: importer package, models & migration (Phases 1–2) - #9
Draft
Willyfrog wants to merge 5 commits into
Draft
Confluence page import: importer package, models & migration (Phases 1–2)#9Willyfrog wants to merge 5 commits into
Willyfrog wants to merge 5 commits into
Conversation
Implements the foundation of the restartable, report-driven Confluence v2
bundle importer described in implementation-plans/confluence-page-import.md.
Phase 1 — pure importer package (server/importer), no HTTP/DB/plugin deps:
- contract.go: v2 producer JSONL DTOs (version/space/page/page_comment/
resolve lines) mirroring mmetl's LineImportData.
- archive.go: secure ZIP inspection with named limit constants; rejects
traversal, backslashes, absolute/drive paths, symlinks, encrypted and
unsupported-method entries, and duplicate raw/normalized names; requires
exactly one root import.jsonl and import-manifest.json; permits but never
opens data/; enforces decompressed size limits while reading.
- inspect.go: strict v2 JSONL sequence/count/hierarchy validation, JSONL
checksum verification against the manifest, independent depth/cycle checks
(depth <= 10), page normalization into StagedPage, count reconciliation,
restricted-page intersection, and stable inspection issue codes.
- tiptap.go: TipTap validation, deterministic compact canonicalization,
SearchText extraction (block separators, hard breaks, whitespace collapse),
and placeholder link discovery limited to approved attrs.
- hash.go: versioned canonical source/applied-state SHA-256 hashing, stable
across map key order; 64-lowercase-hex validation.
- links.go: Confluence placeholder classification (page id/title/file/attachment).
- Full unit-test suite; go test ./server/importer/... passes.
Phase 2 (models + migration):
- model/import.go, model/import_report.go: persisted structs, API-safe views,
state/action/target/mode enums matching the DB CHECK constraints, mandatory
page-only fidelity disclosure, and IsValid validation. Unit tested.
- store/migrations/000005_create_imports.{up,down}.sql: DOCS_ImportSource,
DOCS_ImportJob, DOCS_ImportStagedPage, DOCS_ImportEntity, DOCS_ImportIssue,
DOCS_ImportResult with the plan's indexes and constraints. Applies cleanly
against the Postgres test database via the existing store test harness.
Remaining phases (store CRUD/claiming, worker, HTTP API, webapp wizard) are
not yet implemented; see the PR description.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses confirmed review findings:
1. Placeholder discovery: the mmetl producer emits braced tokens
({{CONF_PAGE_ID:101}} etc.), but classifyPlaceholder only matched the bare
prefix, so real links/images were never discovered. Rewrote links.go to
match the producer's "{{CONF_...:target}}" form via regex (link/attachment
kinds), and detect any "{{CONF_...}}" token in ordinary text. Fixed the
test fixtures to use the real braced format.
2. Hierarchy depth off-by-one: verifyHierarchy counted the root as depth 0 and
accepted an 11-page chain, which the app layer (MaxPageDepth=10, root=1)
rejects at execution. Now counts root as depth 1 (new exported
MaxHierarchyDepth const) so inspection matches execution; corrected the
depth tests (10-page chain allowed, 11-page rejected).
3. Unbounded issue allocation: a valid sub-2 MiB manifest could carry hundreds
of thousands of warnings, each materialized into an issue. Cap copied
warnings at MaxManifestWarnings (1000) and emit one aggregate suppression
issue for the remainder.
5. Overlong titles: normalizePage only checked for an empty title, so a title
over PageTitleMaxRunes passed staging and failed later at execution. Reject
it during inspection with page_title_too_long.
6. Structurally invalid TipTap: walkNode accepted nodes with no type and text
nodes with no text field. Now requires every node to carry a non-empty
string type (tiptap_missing_type) and every text node to have a string text
field (tiptap_bad_text). Unknown type values are still preserved.
7. Declared model limits unenforced: ImportJob.IsValid / ImportSource.IsValid
ignored the display-name/space-title/error-code length constants, so
over-long values failed only at the DB VARCHAR bound. Enforce them at the
model boundary and add ImportIssueRecord.IsValid (stage/severity/code
length/message), which also consumes the previously-dead
ImportIssueCodeMaxRunes constant.
Added unit tests for each fix. go test ./server/... , go build ./... , and
golangci-lint on the changed packages all pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Renumber the import migration and fix seven inspector/model hardening issues that are independent of PR #5: - Migration renumbered 000005 -> 000006 to avoid colliding with PR #5's 000005_add_draft_lastactiveat_baseeditat (morph keys on the version number, so two 000005s would block plugin activation). Updated the model comment and the implementation plan references. archive.go: - Validate mode, encryption, and compression method for every file entry, including data/ payloads that are never opened (previously method/encryption were checked only for import.jsonl/import-manifest.json). - Genuinely normalize entry names via path.Clean before duplicate detection (after the raw ".." check), so "data//x" and "data/x" collide as duplicates instead of the "normalized" map being a no-op alias of the raw map. inspect.go: - Reject a manifest with trailing data after its JSON object (decoder stopped at the first value). - Reject a JSONL line that carries a payload not matching its declared type (e.g. type:"page" also carrying a "space" payload). - Reject a bundle whose manifest source has no space key, since it becomes the ImportSource's required ExternalSpaceKey. - Use attachments_not_imported (plan section 20.2) for the attachment-records issue, distinct from the attachment_placeholder_not_imported link code. - Judge future timestamps against InspectOptions.Now + a skew allowance when supplied (fixed year-2100 ceiling as the pure-function fallback). - Include the manifest advisory target team in the aggregate team-mismatch check. model/import.go: - Require BundleSha256 to be a valid 64-hex digest (never empty) at the model boundary; a persisted job always has it from inspection. Added unit tests for each. go test ./server/... , go build ./... , and golangci-lint on the changed packages all pass; the renamed migration applies cleanly via the store test harness. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…symlink dirs, counts, escaped placeholders, title normalization)
1. Trailing-data checks (tiptap.go, inspect.go parseManifest): json.Decoder.More()
returns false before a closing "]"/"}" delimiter, so "{...}]" and "{...}}"
were accepted and canonicalized. Replaced with a second Decode requiring
io.EOF (the pattern already used in api.go), which rejects any trailing token
while still tolerating trailing whitespace.
2. Expose preflight revision (model/import_report.go): added Revision to
ImportReportSummary so a client can build a valid confirmation request from
the public projection without access to the internal job model. Populated on
the preflight summary only.
3. Symlink directory entries (archive.go): an entry whose name ends in "/"
skipped the mode/symlink check, so a symlink named "data/" was accepted.
Now reject the symlink mode bit for every entry up front, before the
directory exemption for the regular-file/method checks.
4. Manifest count reconciliation (inspect.go): a checksum-valid JSONL whose
parsed page/comment/attachment counts disagree with the manifest is now
rejected (InspectErrCountMismatch) rather than warned. The producer writes
exactly one line per counted entity, so a mismatch can only mean a corrupt
bundle and can never reject a well-formed one. Removed the now-unused
warning issue code.
6. Escaped placeholder braces (links.go): the producer escapes literal braces
in placeholder targets ("{"->"\{", "}"->"\}"). The old "[^}]*" target
stopped at the first escaped "}", so links to titles/attachments containing
braces were omitted from discovery. The target pattern now matches escaped
braces (RE2-compatible alternation) and the captured target is unescaped
back to literal form; applied to both the link and any-token regexes.
7. Title normalization (inspect.go): imported titles were trimmed but not run
through mmmodel.SanitizeUnicode, unlike Page.PreSave. Since import bypasses
PreSave, unsafe Unicode controls could be staged/stored and the source hash
was computed on the unnormalized title (causing spurious reimport conflicts).
Now SanitizeUnicode-then-trim, matching PreSave, feeding the normalized title
into the length check, staging, and the hash.
Added unit tests for each. go test ./server/..., go build ./..., and
golangci-lint on the changed packages all pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…stence 1. Valid large confirmations exceeded StringInterface's hidden 1 MiB limit (model/import.go). mmmodel.StringInterface.Value() rejects any marshaled JSON over maxPropSizeBytes (1 MiB), but a valid confirmation can approve up to 5,000 conflict overwrite descriptors (~350 bytes each ≈ ~1.75 MiB), so it could never be persisted. Introduced a dedicated ImportConfirmation type (raw JSON, driver.Valuer/sql.Scanner) with its own deliberately higher bound (ImportConfirmationMaxBytes = 4 MiB) sized for that worst case, and switched ImportJob.Confirmation to it. The type is documented with the worst-case math and the note that the Phase-4 confirm handler must cap the request body to the same bound (Value() is only the last-line backstop). ImportJob.IsValid now rejects an over-cap confirmation. The bundle summaries keep using StringInterface (fixed-shape count structures, never near 1 MiB). 2. NUL characters passed inspection but PostgreSQL cannot store them (importer). A TipTap text node, title, author id, user proposal, or import_labels prop containing a decoded NUL (U+0000) would fail the staging insert — TEXT columns reject a raw NUL, JSONB rejects the escaped-NUL code point — even though the bundle inspected cleanly (SanitizeUnicode does not drop NUL). Added stripNUL / stripNULFromValue helpers and apply them to SearchText normalization, the title, the author account id, the user proposal, and (recursively) the source-props map. Stripping runs before hashing so the source hash matches the stored value. CanonicalBody was already safe (json.Marshal escapes NUL to a literal \u escape valid in TEXT). Added unit tests: ImportConfirmation Value/Scan round-trip incl. a >1 MiB payload that StringInterface would reject and an over-cap rejection; IsValid over-cap case; and an inspection test asserting NUL is stripped from title, search text, author id, user proposal, and import_labels while surrounding characters survive. go test ./server/..., go build ./..., and golangci-lint all pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Foundation for the restartable, report-driven Confluence v2 bundle page importer specified in
implementation-plans/confluence-page-import.md. This PR delivers the self-contained, fully unit-tested lower layers; the orchestration layers (store CRUD, worker, HTTP API, webapp) are follow-ups tracked below.What's implemented (and verified)
Phase 1 — pure
server/importerpackage (no HTTP/DB/plugin deps)contract.go— v2 producer JSONL DTOs mirroring mmetl'sLineImportData(version/space/page/page_comment/resolve).archive.go— secure ZIP inspection with all limits as named constants. Rejects path traversal, backslashes, absolute/drive-prefixed paths,./..segments, symlinks/non-regular entries, encrypted entries, unsupported compression methods, and duplicate raw/normalized names. Requires exactly one rootimport.jsonl+import-manifest.json; permits but never opensdata/; enforces decompressed size limits while reading (not from ZIP metadata).inspect.go— strict v2 JSONL sequence/count/hierarchy state machine; JSONL SHA-256 verified against the manifest before a job would be created; independent depth (≤10) and cycle checks that do not trust producer flattening; page normalization intoStagedPage; count reconciliation; restricted-page intersection (emitted vs manifest-only); stable inspection issue codes.tiptap.go— TipTap validation, deterministic compact canonicalization (stable for hashing), SearchText extraction (block separators, hard breaks, whitespace/newline collapsing), and placeholder link discovery restricted to linkhref/ imagesrc(text placeholders flagged separately, never rewritten in V1).hash.go— versioned canonical source-state / applied-state SHA-256 hashing, stable across map key order; 64-lowercase-hex validation.links.go— Confluence placeholder classification.Phase 2 (partial) — models + migration
model/import.go,model/import_report.go— persisted structs, API-safeImportJobViewprojections (claim tokens / lease owners / bodies / raw props excluded), enums matching every DBCHECK, the mandatory page-only fidelity disclosure (full_fidelityalways false), andIsValidvalidation. Unit tested.store/migrations/000005_create_imports.{up,down}.sql—DOCS_ImportSource,DOCS_ImportJob,DOCS_ImportStagedPage,DOCS_ImportEntity,DOCS_ImportIssue,DOCS_ImportResultwith the plan's indexes/constraints (incl. the partial unique active-target-execution index and per-source active-job index).VARCHAR(64)for all hash columns per the plan's CHAR-padding warning.Ticket Link
https://mattermost.atlassian.net/browse/MM-69986
Verification
go test ./server/importer/... ./server/model/...— passgo build ./...— cleangolangci-lint run ./server/importer/... ./server/model/...— 0 issues000005applies cleanly against the Postgres test database via the existing store test harness (an existing store test was run and passed, which runsRunMigrationsover all five migrations).Not yet implemented (follow-up PRs, per the plan's sequence)
SKIP LOCKEDclaiming/lease/heartbeat, CAS transitions, source-queue promotion, cleanup,ApplyImportedPage.The importer package and models are stable interfaces the remaining phases build on.
🤖 Generated with Claude Code