From 5004d3a473c7ca54d6126bfe60fe9cc39307ae74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20Vay=C3=A1?= Date: Thu, 23 Jul 2026 17:47:49 +0200 Subject: [PATCH 1/5] =?UTF-8?q?MM-XXXXX:=20Confluence=20page=20import=20?= =?UTF-8?q?=E2=80=94=20importer=20package=20+=20models=20+=20migration?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- server/importer/archive.go | 268 +++++++ server/importer/contract.go | 145 ++++ server/importer/hash.go | 74 ++ server/importer/hash_test.go | 73 ++ server/importer/inspect.go | 697 ++++++++++++++++++ server/importer/inspect_test.go | 462 ++++++++++++ server/importer/links.go | 77 ++ server/importer/testhelpers_test.go | 170 +++++ server/importer/tiptap.go | 253 +++++++ server/importer/tiptap_test.go | 182 +++++ server/model/import.go | 448 +++++++++++ server/model/import_report.go | 118 +++ server/model/import_test.go | 128 ++++ .../migrations/000005_create_imports.down.sql | 7 + .../migrations/000005_create_imports.up.sql | 222 ++++++ 15 files changed, 3324 insertions(+) create mode 100644 server/importer/archive.go create mode 100644 server/importer/contract.go create mode 100644 server/importer/hash.go create mode 100644 server/importer/hash_test.go create mode 100644 server/importer/inspect.go create mode 100644 server/importer/inspect_test.go create mode 100644 server/importer/links.go create mode 100644 server/importer/testhelpers_test.go create mode 100644 server/importer/tiptap.go create mode 100644 server/importer/tiptap_test.go create mode 100644 server/model/import.go create mode 100644 server/model/import_report.go create mode 100644 server/model/import_test.go create mode 100644 server/store/migrations/000005_create_imports.down.sql create mode 100644 server/store/migrations/000005_create_imports.up.sql diff --git a/server/importer/archive.go b/server/importer/archive.go new file mode 100644 index 0000000..bd945d3 --- /dev/null +++ b/server/importer/archive.go @@ -0,0 +1,268 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package importer + +import ( + "archive/zip" + "crypto/sha256" + "encoding/hex" + "fmt" + "io" + "io/fs" + "strings" +) + +// fsModeSymlink is the file-mode bit marking a symlink entry. +const fsModeSymlink = fs.ModeSymlink + +// First-release archive/content limits. Kept as named constants (not magic numbers in handlers) +// so the upload handler and the archive inspector share one source of truth. +const ( + // MaxArchiveEntries bounds the number of entries in the ZIP central directory. + MaxArchiveEntries = 25_000 + // MaxManifestBytes bounds the decompressed import-manifest.json. + MaxManifestBytes = 2 * 1024 * 1024 + // MaxJSONLBytes bounds the decompressed import.jsonl. + MaxJSONLBytes = 128 * 1024 * 1024 + // MaxJSONLLineBytes bounds a single JSONL line. + MaxJSONLLineBytes = 8 * 1024 * 1024 + // MaxPages bounds the number of page lines. + MaxPages = 5_000 + // MaxTipTapNodes bounds the number of nodes in a single page's TipTap document. + MaxTipTapNodes = 250_000 + // MaxTipTapDepth bounds TipTap nesting depth. + MaxTipTapDepth = 100 +) + +// Fixed entry names required at the archive root. +const ( + entryJSONL = "import.jsonl" + entryManifest = "import-manifest.json" + entryDataDir = "data/" +) + +// ArchiveContents holds the decompressed bytes of the two required root entries plus the +// SHA-256 the caller computed over the archive as a whole. Attachment bytes under data/ are never +// read, so they are absent here. +type ArchiveContents struct { + ManifestBytes []byte + JSONLBytes []byte + // JSONLSha256 is the lowercase hex SHA-256 of the exact decompressed import.jsonl bytes. + JSONLSha256 string + // HasDataDir reports whether any entry under data/ existed (metadata only; never extracted). + HasDataDir bool +} + +// ArchiveError describes a rejected archive with a stable code so callers can map it to a +// user-facing message and HTTP status. +type ArchiveError struct { + Code string + Message string +} + +func (e *ArchiveError) Error() string { return e.Message } + +func archiveErr(code, format string, args ...any) *ArchiveError { + return &ArchiveError{Code: code, Message: fmt.Sprintf(format, args...)} +} + +// Stable archive rejection codes. +const ( + ArchiveErrUnreadable = "archive_unreadable" + ArchiveErrTooManyEntries = "archive_too_many_entries" + ArchiveErrBadEntryName = "archive_bad_entry_name" + ArchiveErrDuplicateEntry = "archive_duplicate_entry" + ArchiveErrUnsafeEntry = "archive_unsafe_entry" + ArchiveErrEncryptedEntry = "archive_encrypted_entry" + ArchiveErrUnsupportedMethod = "archive_unsupported_method" + ArchiveErrMissingJSONL = "archive_missing_jsonl" + ArchiveErrMissingManifest = "archive_missing_manifest" + ArchiveErrUnexpectedEntry = "archive_unexpected_entry" + ArchiveErrManifestTooLarge = "archive_manifest_too_large" + ArchiveErrJSONLTooLarge = "archive_jsonl_too_large" +) + +// InspectArchive validates the structure and safety of a ZIP bundle read from r (of size n) and +// returns the decompressed manifest and JSONL bytes. It never extracts or opens data/ files. +// +// Structural rules (before opening any body): reject duplicate raw or normalized names, +// backslash separators, empty names, NUL bytes, absolute paths, Windows drive prefixes, "."/".." +// segments, symlinks/non-regular entries, encrypted entries, and unsupported compression methods. +// Require exactly one root import.jsonl and one root import-manifest.json; permit other files only +// below data/. +func InspectArchive(r io.ReaderAt, n int64) (*ArchiveContents, error) { + zr, err := zip.NewReader(r, n) + if err != nil { + return nil, archiveErr(ArchiveErrUnreadable, "failed to read zip archive: %v", err) + } + if len(zr.File) > MaxArchiveEntries { + return nil, archiveErr(ArchiveErrTooManyEntries, "archive has %d entries, limit is %d", len(zr.File), MaxArchiveEntries) + } + + rawSeen := make(map[string]struct{}, len(zr.File)) + normSeen := make(map[string]struct{}, len(zr.File)) + + var jsonlFile, manifestFile *zip.File + hasDataDir := false + + for _, f := range zr.File { + raw := f.Name + if raw == "" { + return nil, archiveErr(ArchiveErrBadEntryName, "archive contains an empty entry name") + } + if strings.ContainsRune(raw, 0) { + return nil, archiveErr(ArchiveErrBadEntryName, "archive entry name contains a NUL byte") + } + if strings.Contains(raw, "\\") { + // Reject rather than silently converting: a backslash in a ZIP name is either a + // non-conformant producer or an evasion attempt. + return nil, archiveErr(ArchiveErrBadEntryName, "archive entry %q contains a backslash", raw) + } + if _, dup := rawSeen[raw]; dup { + return nil, archiveErr(ArchiveErrDuplicateEntry, "archive contains duplicate entry %q", raw) + } + rawSeen[raw] = struct{}{} + + name := raw // only "/" separators are permitted, verified above + + if unsafeErr := checkUnsafeName(name); unsafeErr != nil { + return nil, unsafeErr + } + if _, dup := normSeen[name]; dup { + return nil, archiveErr(ArchiveErrDuplicateEntry, "archive contains duplicate normalized entry %q", name) + } + normSeen[name] = struct{}{} + + isDir := strings.HasSuffix(name, "/") + if !isDir { + if modeErr := checkEntryMode(f); modeErr != nil { + return nil, modeErr + } + } + + switch { + case name == entryJSONL: + if methodErr := checkSupportedMethod(f); methodErr != nil { + return nil, methodErr + } + jsonlFile = f + case name == entryManifest: + if methodErr := checkSupportedMethod(f); methodErr != nil { + return nil, methodErr + } + manifestFile = f + case name == entryDataDir || strings.HasPrefix(name, entryDataDir): + // data/ files are permitted but never opened. + hasDataDir = true + default: + return nil, archiveErr(ArchiveErrUnexpectedEntry, "unexpected archive entry %q; only import.jsonl, import-manifest.json, and data/ are allowed", name) + } + } + + if jsonlFile == nil { + return nil, archiveErr(ArchiveErrMissingJSONL, "archive is missing required import.jsonl") + } + if manifestFile == nil { + return nil, archiveErr(ArchiveErrMissingManifest, "archive is missing required import-manifest.json") + } + + manifestBytes, err := readLimited(manifestFile, MaxManifestBytes) + if err != nil { + if err == errTooLarge { + return nil, archiveErr(ArchiveErrManifestTooLarge, "import-manifest.json exceeds %d bytes", MaxManifestBytes) + } + return nil, archiveErr(ArchiveErrUnreadable, "failed to read import-manifest.json: %v", err) + } + + jsonlBytes, err := readLimited(jsonlFile, MaxJSONLBytes) + if err != nil { + if err == errTooLarge { + return nil, archiveErr(ArchiveErrJSONLTooLarge, "import.jsonl exceeds %d bytes", MaxJSONLBytes) + } + return nil, archiveErr(ArchiveErrUnreadable, "failed to read import.jsonl: %v", err) + } + + sum := sha256.Sum256(jsonlBytes) + + return &ArchiveContents{ + ManifestBytes: manifestBytes, + JSONLBytes: jsonlBytes, + JSONLSha256: hex.EncodeToString(sum[:]), + HasDataDir: hasDataDir, + }, nil +} + +// checkUnsafeName rejects path traversal and platform-specific unsafe names on a "/"-separated +// entry name. +func checkUnsafeName(name string) error { + if strings.HasPrefix(name, "/") { + return archiveErr(ArchiveErrUnsafeEntry, "archive entry %q is an absolute path", name) + } + // Windows drive prefix such as "C:". + if len(name) >= 2 && name[1] == ':' { + return archiveErr(ArchiveErrUnsafeEntry, "archive entry %q has a drive prefix", name) + } + for seg := range strings.SplitSeq(name, "/") { + if seg == "." || seg == ".." { + return archiveErr(ArchiveErrUnsafeEntry, "archive entry %q contains a %q segment", name, seg) + } + } + return nil +} + +// checkEntryMode rejects symlinks and other non-regular file entries. Directory entries are +// handled by the caller (trailing "/"). +func checkEntryMode(f *zip.File) error { + mode := f.Mode() + if mode&fsModeSymlink != 0 { + return archiveErr(ArchiveErrUnsafeEntry, "archive entry %q is a symlink", f.Name) + } + if !mode.IsRegular() { + return archiveErr(ArchiveErrUnsafeEntry, "archive entry %q is not a regular file", f.Name) + } + return nil +} + +// checkSupportedMethod rejects encrypted entries and unsupported compression methods for entries +// whose bodies will be read (manifest/JSONL). +func checkSupportedMethod(f *zip.File) error { + // Bit 0 of the general-purpose flag marks an encrypted entry. + if f.Flags&0x1 != 0 { + return archiveErr(ArchiveErrEncryptedEntry, "archive entry %q is encrypted", f.Name) + } + if f.Method != zip.Store && f.Method != zip.Deflate { + return archiveErr(ArchiveErrUnsupportedMethod, "archive entry %q uses unsupported compression method %d", f.Name, f.Method) + } + return nil +} + +// errTooLarge is a sentinel used internally by readLimited so callers can distinguish an +// over-limit body from a genuine read failure. +var errTooLarge = fmt.Errorf("entry exceeds decompressed size limit") + +// readLimited decompresses f, enforcing the decompressed byte limit while reading (not trusting +// declared ZIP metadata) rather than the compressed metadata. It reads one byte past the limit to +// detect an exact overflow: an entry within the limit is read to EOF, which makes archive/zip +// verify the entry CRC; an over-limit entry is rejected before it can be fully decompressed, so +// there is no unbounded decompression. +func readLimited(f *zip.File, limit int64) (_ []byte, err error) { + rc, err := f.Open() + if err != nil { + return nil, err + } + defer func() { + if cerr := rc.Close(); cerr != nil && err == nil { + err = cerr + } + }() + + buf, err := io.ReadAll(io.LimitReader(rc, limit+1)) + if err != nil { + return nil, err + } + if int64(len(buf)) > limit { + return nil, errTooLarge + } + return buf, nil +} diff --git a/server/importer/contract.go b/server/importer/contract.go new file mode 100644 index 0000000..daf422e --- /dev/null +++ b/server/importer/contract.go @@ -0,0 +1,145 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +// Package importer contains the pure, side-effect-free logic that consumes an mmetl +// Confluence v2 bundle: secure archive inspection, strict JSONL parsing, TipTap +// canonicalization, SearchText extraction, placeholder link discovery, and canonical +// hashing. It has no HTTP, database, or plugin-API dependencies so it can be unit-tested +// in isolation; the app/store layers orchestrate it. +package importer + +// ContractVersion is the only JSONL contract version this importer accepts. +const ContractVersion = 2 + +// ManifestVersion is the string value the manifest's version field must carry for v2. +const ManifestVersion = "2" + +// Line type discriminators emitted by the producer, one per JSONL line. +const ( + LineTypeVersion = "version" + LineTypeSpace = "space" + LineTypePage = "page" + LineTypePageComment = "page_comment" + LineTypeResolveSpacePlaceholders = "resolve_space_placeholders" +) + +// Line mirrors the producer's LineImportData. Every JSONL line decodes into this shape; the +// payload matching Type is non-nil and the others are nil. Pointer payload fields let the parser +// verify that a line carries exactly the payload its Type declares. Unknown object fields are +// tolerated (forward-compatible v2 additions) because json.Unmarshal ignores them by default. +type Line struct { + Type string `json:"type"` + Version *int `json:"version,omitempty"` + Source *SourceData `json:"source,omitempty"` + Space *SpaceData `json:"space,omitempty"` + Page *PageData `json:"page,omitempty"` + PageComment *PageCommentData `json:"page_comment,omitempty"` + ResolveSpacePlaceholders *ResolvePlaceholdersData `json:"resolve_space_placeholders,omitempty"` +} + +// SourceData is the bundle's source namespace, carried once on the version line and mirrored in +// the manifest. OrganizationID is optional metadata; SpaceKey scopes bare source IDs. +type SourceData struct { + OrganizationID *string `json:"organization_id,omitempty"` + SpaceKey *string `json:"space_key,omitempty"` +} + +// SpaceData is the single space line. Team is advisory; Props carries import_source_id. +type SpaceData struct { + Team *string `json:"team"` + Title *string `json:"title,omitempty"` + Description *string `json:"description,omitempty"` + Props *map[string]any `json:"props,omitempty"` +} + +// PageData is one page line. Content is a JSON string holding TipTap JSON that must be decoded and +// validated independently of the line. The page's bare external ID is derived only from +// Props["import_source_id"], never from any other field. +type PageData struct { + Team *string `json:"team"` + SpaceImportSourceID *string `json:"space_import_source_id"` + User *string `json:"user"` + Title *string `json:"title"` + Content *string `json:"content"` + ParentImportSourceID *string `json:"parent_import_source_id,omitempty"` + CreateAt *int64 `json:"create_at,omitempty"` + UpdateAt *int64 `json:"update_at,omitempty"` + Props *map[string]any `json:"props,omitempty"` + Attachments *[]AttachmentData `json:"attachments,omitempty"` +} + +// PageCommentData is one comment line. Comments are parsed and counted but never staged as +// individual entities in this release. +type PageCommentData struct { + PageImportSourceID *string `json:"page_import_source_id"` + ParentCommentImportSourceID *string `json:"parent_comment_import_source_id,omitempty"` + User *string `json:"user"` + Content *string `json:"content"` + CreateAt *int64 `json:"create_at,omitempty"` + UpdateAt *int64 `json:"update_at,omitempty"` + IsResolved *bool `json:"is_resolved,omitempty"` + Props *map[string]any `json:"props,omitempty"` +} + +// AttachmentData is one attachment metadata entry. Its bytes are never opened in this release; only +// its path is validated and it is counted. +type AttachmentData struct { + Path *string `json:"path"` + Props *map[string]any `json:"props,omitempty"` +} + +// ResolvePlaceholdersData is the trailing resolve_space_placeholders line. +type ResolvePlaceholdersData struct { + Team *string `json:"team"` + SpaceImportSourceID *string `json:"space_import_source_id"` +} + +// Well-known page prop keys emitted by the producer inside PageData.Props. +const ( + PropImportSourceID = "import_source_id" + PropImportSource = "import_source" + PropConfluenceSpaceKey = "confluence_space_key" + PropConfluenceAuthorAccountID = "confluence_author_account_id" + PropImportLabels = "import_labels" +) + +// AllowlistedSourceProps names the page source props copied verbatim into the docs_import +// namespace on execution. Arbitrary future producer fields are deliberately not copied. +var AllowlistedSourceProps = []string{PropImportLabels} + +// stringOrEmpty dereferences a *string, returning "" for nil. +func stringOrEmpty(s *string) string { + if s == nil { + return "" + } + return *s +} + +// int64OrZero dereferences a *int64, returning 0 for nil. +func int64OrZero(v *int64) int64 { + if v == nil { + return 0 + } + return *v +} + +// propString reads a string-valued prop from an untyped props map, returning "" when absent or of +// a non-string type. Using json.Number-safe handling is unnecessary here: string props decode as +// Go strings regardless of UseNumber. +func propString(props map[string]any, key string) string { + if props == nil { + return "" + } + if v, ok := props[key].(string); ok { + return v + } + return "" +} + +// derefProps returns the props map behind a *map pointer, or nil. +func derefProps(p *map[string]any) map[string]any { + if p == nil { + return nil + } + return *p +} diff --git a/server/importer/hash.go b/server/importer/hash.go new file mode 100644 index 0000000..afad4a4 --- /dev/null +++ b/server/importer/hash.go @@ -0,0 +1,74 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package importer + +import ( + "crypto/sha256" + "encoding/hex" + "encoding/json" + "regexp" +) + +// HashFormatVersion versions the hash input shapes so a later deliberate change can migrate +// baselines rather than silently invalidating every mapping. +const HashFormatVersion = 1 + +// SourceStateHashInput is the canonical shape hashed to detect whether a page's source content +// changed between imports. It deliberately excludes bundle team, job ID, target IDs, +// attachment/comment counts, and source ordinal — none of which are page content. +type SourceStateHashInput struct { + Version int `json:"version"` + Title string `json:"title"` + CanonicalBody string `json:"canonical_body"` + ParentExternalID string `json:"parent_external_id"` + AuthorAccountID string `json:"author_account_id"` + AuthorProposal string `json:"author_proposal"` + SourceCreateAt int64 `json:"source_create_at"` + SourceUpdateAt int64 `json:"source_update_at"` + SourceProps map[string]any `json:"source_props"` +} + +// AppliedStateHashInput is the canonical shape hashed to detect whether the local page was edited +// after the last import. It excludes SearchText (derived from Body), numeric SortOrder, timestamps, +// and modifier identity — all of which change through normal editing unrelated to content. +type AppliedStateHashInput struct { + Version int `json:"version"` + Title string `json:"title"` + CanonicalBody string `json:"canonical_body"` + ParentID string `json:"parent_id"` + DocsImportSourceFields map[string]any `json:"docs_import_source_fields"` +} + +// HashSourceState returns the lowercase-hex SHA-256 of the canonical source-state input. +func HashSourceState(in SourceStateHashInput) (string, error) { + in.Version = HashFormatVersion + return canonicalHashHex(in) +} + +// HashAppliedState returns the lowercase-hex SHA-256 of the canonical applied-state input. +func HashAppliedState(in AppliedStateHashInput) (string, error) { + in.Version = HashFormatVersion + return canonicalHashHex(in) +} + +// canonicalHashHex marshals v to canonical JSON (Go sorts object keys, including nested map keys, +// so key order in the source props is irrelevant) and returns the SHA-256 as lowercase hex. +func canonicalHashHex(v any) (string, error) { + b, err := json.Marshal(v) + if err != nil { + return "", err + } + sum := sha256.Sum256(b) + return hex.EncodeToString(sum[:]), nil +} + +// hexSHA256Pattern matches exactly 64 lowercase hexadecimal characters. +var hexSHA256Pattern = regexp.MustCompile(`^[0-9a-f]{64}$`) + +// IsValidSHA256Hex reports whether s is exactly 64 lowercase hexadecimal characters. Enforced at +// the model/application boundary for every non-empty hash so a CHAR-padded or malformed value +// never enters a comparison. +func IsValidSHA256Hex(s string) bool { + return hexSHA256Pattern.MatchString(s) +} diff --git a/server/importer/hash_test.go b/server/importer/hash_test.go new file mode 100644 index 0000000..dec0a5d --- /dev/null +++ b/server/importer/hash_test.go @@ -0,0 +1,73 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package importer + +import "testing" + +func TestHashSourceState_StableAcrossMapKeyOrder(t *testing.T) { + a := SourceStateHashInput{ + Title: "Title", + CanonicalBody: `{"type":"doc"}`, + SourceProps: map[string]any{ + "import_labels": []any{"x", "y"}, + "zzz": "last", + "aaa": "first", + }, + } + b := SourceStateHashInput{ + Title: "Title", + CanonicalBody: `{"type":"doc"}`, + SourceProps: map[string]any{ + "aaa": "first", + "zzz": "last", + "import_labels": []any{"x", "y"}, + }, + } + ha, err := HashSourceState(a) + if err != nil { + t.Fatal(err) + } + hb, err := HashSourceState(b) + if err != nil { + t.Fatal(err) + } + if ha != hb { + t.Errorf("hash not stable across map key order: %s != %s", ha, hb) + } + if !IsValidSHA256Hex(ha) { + t.Errorf("hash not 64-hex: %q", ha) + } +} + +func TestHashSourceState_ChangesWithContent(t *testing.T) { + base := SourceStateHashInput{Title: "A", CanonicalBody: `{"type":"doc"}`} + changed := base + changed.CanonicalBody = `{"type":"doc","content":[]}` + h1, _ := HashSourceState(base) + h2, _ := HashSourceState(changed) + if h1 == h2 { + t.Errorf("expected different hashes for different bodies") + } +} + +func TestHashAppliedState_Deterministic(t *testing.T) { + in := AppliedStateHashInput{Title: "T", CanonicalBody: `{"type":"doc"}`, ParentID: "abc"} + h1, _ := HashAppliedState(in) + h2, _ := HashAppliedState(in) + if h1 != h2 || !IsValidSHA256Hex(h1) { + t.Errorf("applied hash not deterministic/valid: %s %s", h1, h2) + } +} + +func TestIsValidSHA256Hex(t *testing.T) { + valid := "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + if !IsValidSHA256Hex(valid) { + t.Errorf("expected valid") + } + for _, bad := range []string{"", "ABCDEF", valid + "0", valid[:63], "z" + valid[1:]} { + if IsValidSHA256Hex(bad) { + t.Errorf("expected invalid: %q", bad) + } + } +} diff --git a/server/importer/inspect.go b/server/importer/inspect.go new file mode 100644 index 0000000..1b8cbfa --- /dev/null +++ b/server/importer/inspect.go @@ -0,0 +1,697 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package importer + +import ( + "encoding/json" + "fmt" + "strings" +) + +// Manifest mirrors the fields of the producer's import-manifest.json that the importer reads. +// Unknown fields are ignored (forward-compatible). +type Manifest struct { + Version string `json:"version"` + Source ManifestSource `json:"source"` + Target ManifestTarget `json:"target"` + Counts ManifestCounts `json:"counts"` + Checksums ManifestChecksums `json:"checksums"` + Users []ManifestUser `json:"users"` + RestrictedPages []ManifestRestrictedPage `json:"restricted_pages"` + Warnings []string `json:"warnings"` + Errors []string `json:"errors"` +} + +// ManifestSource is the bundle's source namespace metadata. +type ManifestSource struct { + Type string `json:"type"` + OrganizationID string `json:"organization_id"` + SpaceKey string `json:"space_key"` + SpaceName string `json:"space_name"` + ExportFile string `json:"export_file"` +} + +// ManifestTarget carries advisory destination metadata (never authoritative). +type ManifestTarget struct { + Team string `json:"team"` +} + +// ManifestCounts holds the producer's entity counts, reconciled against parsed values. +type ManifestCounts struct { + Spaces int `json:"spaces"` + Pages int `json:"pages"` + Comments int `json:"comments"` + Attachments int `json:"attachments"` +} + +// ManifestChecksums carries the JSONL checksum used to verify archive integrity. +type ManifestChecksums struct { + JSONLSha256 string `json:"jsonl_sha256"` + AttachmentsSha256 string `json:"attachments_sha256"` +} + +// ManifestUser maps a source Confluence account to a proposed Mattermost username. +type ManifestUser struct { + AccountID string `json:"account_id"` + ConfluenceUsername string `json:"confluence_username"` + MattermostUsername string `json:"mattermost_username"` +} + +// ManifestRestrictedPage records a page that carried a Confluence view restriction. +type ManifestRestrictedPage struct { + ID string `json:"id"` + Title string `json:"title"` +} + +// InspectError is a hard failure that prevents a job from being created. It carries a stable code. +type InspectError struct { + Code string + Message string +} + +func (e *InspectError) Error() string { return e.Message } + +func inspectErr(code, format string, args ...any) *InspectError { + return &InspectError{Code: code, Message: fmt.Sprintf(format, args...)} +} + +// Stable inspection hard-failure codes. +const ( + InspectErrManifestInvalid = "manifest_invalid" + InspectErrManifestVersion = "manifest_unsupported_version" + InspectErrManifestHasErrors = "manifest_reports_errors" + InspectErrChecksumMissing = "jsonl_checksum_missing" + InspectErrChecksumMismatch = "jsonl_checksum_mismatch" + InspectErrJSONLEmpty = "jsonl_empty" + InspectErrBlankLine = "jsonl_blank_line" + InspectErrLineTooLong = "jsonl_line_too_long" + InspectErrLineInvalid = "jsonl_line_invalid" + InspectErrSequence = "jsonl_bad_sequence" + InspectErrUnknownType = "jsonl_unknown_type" + InspectErrPayloadMismatch = "jsonl_payload_mismatch" + InspectErrVersionValue = "jsonl_unsupported_version" + InspectErrTooManyPages = "jsonl_too_many_pages" + InspectErrPageMissingID = "page_missing_external_id" + InspectErrDuplicatePageID = "page_duplicate_external_id" + InspectErrPageMissingTitle = "page_missing_title" + InspectErrParentNotSeen = "page_parent_not_seen" + InspectErrCycle = "page_cycle" + InspectErrDepthExceeded = "page_depth_exceeded" + InspectErrTipTap = "page_content_invalid" + InspectErrSpaceKeyMismatch = "space_key_mismatch" + InspectErrCommentMissingPageID = "comment_missing_page_id" + InspectErrAttachmentPath = "attachment_invalid_path" + InspectErrHash = "hash_failed" +) + +// Inspection issue severities. +const ( + SeverityInfo = "info" + SeverityWarning = "warning" + SeverityError = "error" +) + +// Stable inspection-stage issue codes (non-fatal; surfaced in the report). +const ( + IssueBundleTeamMismatch = "bundle_team_mismatch" + IssueManifestWarning = "manifest_warning" + IssueAttachmentChecksumNotVerified = "attachment_checksum_not_verified" + IssueSourceCreateAtInvalid = "source_create_at_invalid" + IssueSourceUpdateAtInvalid = "source_update_at_invalid" + IssueManifestCountMismatch = "manifest_count_mismatch" + IssuePlaceholderInText = "placeholder_in_text_not_rewritten" + IssueAttachmentNotImported = "attachment_placeholder_not_imported" +) + +// InspectionIssue is one non-fatal finding recorded during inspection. +type InspectionIssue struct { + Severity string `json:"severity"` + Code string `json:"code"` + ExternalID string `json:"external_id,omitempty"` + Title string `json:"title,omitempty"` + Message string `json:"message"` + Remediation string `json:"remediation,omitempty"` + Details map[string]any `json:"details,omitempty"` +} + +// StagedPage is one normalized page ready to persist to DOCS_ImportStagedPage. It holds no HTTP or +// DB types; the store maps it to a row. +type StagedPage struct { + Ordinal int + ExternalID string + ParentExternalID string + SourceOrdinal int + Title string + CanonicalBody string + SearchText string + SourceUserProposal string + SourceAuthorAccountID string + SourceCreateAt int64 + SourceUpdateAt int64 + SourceProps map[string]any + IncomingSourceHash string + // Links are the placeholders discovered in this page's approved attributes and text. Not + // persisted verbatim; used for link counts and per-page link issues during preflight. + Links []DiscoveredLink +} + +// RestrictedSummary partitions manifest restricted entries by whether they intersect emitted pages. +type RestrictedSummary struct { + ManifestTotal int `json:"restricted_manifest_total"` + EmittedPages int `json:"restricted_emitted_pages"` + ManifestOnly int `json:"restricted_manifest_only"` + EmittedIDs []string `json:"restricted_emitted_ids,omitempty"` + ManifestOnlyIDs []string `json:"restricted_manifest_only_ids,omitempty"` +} + +// InspectionResult is the complete synchronous inspection output. +type InspectionResult struct { + Version int + OrganizationID string + SpaceKey string + SpaceName string + SpaceTitle string + SpaceDescription string + + Pages []StagedPage + CommentCount int + AttachmentCount int + Restricted RestrictedSummary + + Manifest *Manifest + JSONLSha256 string + + Issues []InspectionIssue +} + +// InspectOptions carries optional context the pure inspector uses only for advisory checks. +type InspectOptions struct { + // RequestedTeamName, when non-empty, is compared against the advisory bundle team values to + // emit a single aggregate bundle_team_mismatch warning. The value is never used to route. + RequestedTeamName string +} + +// parsing states for the JSONL sequence. +type parseState int + +const ( + stateVersion parseState = iota + stateSpace + statePages + stateComments + stateDone +) + +// Inspect performs full synchronous inspection of an already-safely-decompressed bundle: it parses +// and validates the manifest and JSONL, verifies the JSONL checksum, normalizes pages, and +// reconciles counts. A hard failure returns an *InspectError and no result; recoverable findings +// are collected as issues on the result. +func Inspect(contents *ArchiveContents, opts InspectOptions) (*InspectionResult, error) { + manifest, err := parseManifest(contents.ManifestBytes) + if err != nil { + return nil, err + } + + // Verify JSONL integrity before doing anything else with the data. + if manifest.Checksums.JSONLSha256 == "" { + return nil, inspectErr(InspectErrChecksumMissing, "manifest is missing checksums.jsonl_sha256") + } + if !strings.EqualFold(manifest.Checksums.JSONLSha256, contents.JSONLSha256) { + return nil, inspectErr(InspectErrChecksumMismatch, "import.jsonl checksum does not match the manifest") + } + + // A producer that reported its own errors yields a failed upload. + if len(manifest.Errors) > 0 { + return nil, inspectErr(InspectErrManifestHasErrors, "manifest reports %d producer error(s): %s", len(manifest.Errors), manifest.Errors[0]) + } + + res := &InspectionResult{ + Manifest: manifest, + JSONLSha256: contents.JSONLSha256, + OrganizationID: manifest.Source.OrganizationID, + SpaceKey: manifest.Source.SpaceKey, + SpaceName: manifest.Source.SpaceName, + } + + // Copy manifest warnings into issues. + for _, w := range manifest.Warnings { + res.Issues = append(res.Issues, InspectionIssue{ + Severity: SeverityWarning, Code: IssueManifestWarning, Message: w, + Remediation: "Review the producer warning; it does not block import.", + }) + } + // Attachments are never verified in this release. + if manifest.Checksums.AttachmentsSha256 != "" { + res.Issues = append(res.Issues, InspectionIssue{ + Severity: SeverityInfo, Code: IssueAttachmentChecksumNotVerified, + Message: "attachment checksum not verified: attachments are out of scope in this release", + Remediation: "Attachment bytes are neither extracted nor verified; import attachments in a future release.", + Details: map[string]any{"reason": "attachments_out_of_scope"}, + }) + } + + if err := parseJSONL(contents.JSONLBytes, manifest, opts, res); err != nil { + return nil, err + } + + reconcileCounts(manifest, res) + summarizeRestricted(manifest, res) + + return res, nil +} + +// parseManifest decodes and version-checks the manifest. +func parseManifest(b []byte) (*Manifest, error) { + var m Manifest + dec := json.NewDecoder(strings.NewReader(string(b))) + if err := dec.Decode(&m); err != nil { + return nil, inspectErr(InspectErrManifestInvalid, "manifest is not valid JSON: %v", err) + } + if m.Version != ManifestVersion { + return nil, inspectErr(InspectErrManifestVersion, "manifest version %q is unsupported; require %q", m.Version, ManifestVersion) + } + return &m, nil +} + +// parseJSONL runs the strict v2 line sequence state machine, normalizing pages into res. +func parseJSONL(b []byte, manifest *Manifest, opts InspectOptions, res *InspectionResult) error { + lines := splitJSONLLines(b) + if len(lines) == 0 { + return inspectErr(InspectErrJSONLEmpty, "import.jsonl is empty") + } + + state := stateVersion + seenPageIDs := make(map[string]struct{}) + parentOf := make(map[string]string) + siblingCounter := make(map[string]int) // parent external ID ("" for roots) -> next sibling ordinal + teamValues := make(map[string]struct{}) + + pageOrdinal := 0 + + for i, raw := range lines { + if raw == "" { + return inspectErr(InspectErrBlankLine, "import.jsonl line %d is blank", i+1) + } + if len(raw) > MaxJSONLLineBytes { + return inspectErr(InspectErrLineTooLong, "import.jsonl line %d exceeds %d bytes", i+1, MaxJSONLLineBytes) + } + + var line Line + if err := json.Unmarshal([]byte(raw), &line); err != nil { + return inspectErr(InspectErrLineInvalid, "import.jsonl line %d is invalid JSON: %v", i+1, err) + } + + switch line.Type { + case LineTypeVersion: + if state != stateVersion { + return inspectErr(InspectErrSequence, "unexpected version line at line %d", i+1) + } + if line.Version == nil || *line.Version != ContractVersion { + return inspectErr(InspectErrVersionValue, "line %d: version must be %d", i+1, ContractVersion) + } + if line.Source != nil { + if err := requireSameSpaceKey("version.source.space_key", stringOrEmpty(line.Source.SpaceKey), manifest.Source.SpaceKey); err != nil { + return err + } + } + res.Version = *line.Version + state = stateSpace + + case LineTypeSpace: + if state != stateSpace { + return inspectErr(InspectErrSequence, "unexpected space line at line %d", i+1) + } + if line.Space == nil { + return inspectErr(InspectErrPayloadMismatch, "line %d declares type space but has no space payload", i+1) + } + if err := handleSpaceLine(line.Space, manifest, teamValues, res); err != nil { + return err + } + state = statePages + + case LineTypePage: + if state != statePages { + return inspectErr(InspectErrSequence, "unexpected page line at line %d (pages must follow the space line and precede comments)", i+1) + } + if line.Page == nil { + return inspectErr(InspectErrPayloadMismatch, "line %d declares type page but has no page payload", i+1) + } + if len(res.Pages) >= MaxPages { + return inspectErr(InspectErrTooManyPages, "bundle has more than %d pages", MaxPages) + } + sp, err := normalizePage(line.Page, manifest, i+1, pageOrdinal, seenPageIDs, parentOf, siblingCounter, teamValues, res) + if err != nil { + return err + } + res.Pages = append(res.Pages, *sp) + pageOrdinal++ + + case LineTypePageComment: + if state != statePages && state != stateComments { + return inspectErr(InspectErrSequence, "unexpected page_comment line at line %d", i+1) + } + state = stateComments + if line.PageComment == nil { + return inspectErr(InspectErrPayloadMismatch, "line %d declares type page_comment but has no payload", i+1) + } + if stringOrEmpty(line.PageComment.PageImportSourceID) == "" { + return inspectErr(InspectErrCommentMissingPageID, "line %d: page_comment is missing page_import_source_id", i+1) + } + res.CommentCount++ + + case LineTypeResolveSpacePlaceholders: + if state != statePages && state != stateComments { + return inspectErr(InspectErrSequence, "unexpected resolve_space_placeholders line at line %d", i+1) + } + if line.ResolveSpacePlaceholders == nil { + return inspectErr(InspectErrPayloadMismatch, "line %d declares type resolve_space_placeholders but has no payload", i+1) + } + collectTeam(teamValues, stringOrEmpty(line.ResolveSpacePlaceholders.Team)) + state = stateDone + + case "": + return inspectErr(InspectErrUnknownType, "line %d has an empty type", i+1) + default: + return inspectErr(InspectErrUnknownType, "line %d has unknown type %q", i+1, line.Type) + } + } + + if state != stateDone { + return inspectErr(InspectErrSequence, "import.jsonl is missing the trailing resolve_space_placeholders line") + } + + // Independently verify hierarchy depth over the assembled parent map. + if err := verifyHierarchy(res.Pages, parentOf); err != nil { + return err + } + + emitTeamMismatch(teamValues, opts, res) + return nil +} + +// splitJSONLLines splits on newline and drops exactly one trailing terminator newline, so a normal +// file ending in "\n" is not treated as having a blank final line. Any other empty element remains +// and is rejected as a blank line by the caller. +func splitJSONLLines(b []byte) []string { + s := string(b) + if s == "" { + return nil + } + lines := strings.Split(s, "\n") + if len(lines) > 0 && lines[len(lines)-1] == "" { + lines = lines[:len(lines)-1] + } + return lines +} + +// handleSpaceLine records the space defaults and validates the space key against the manifest. +func handleSpaceLine(space *SpaceData, manifest *Manifest, teamValues map[string]struct{}, res *InspectionResult) error { + collectTeam(teamValues, stringOrEmpty(space.Team)) + res.SpaceTitle = stringOrEmpty(space.Title) + res.SpaceDescription = stringOrEmpty(space.Description) + + spaceKey := propString(derefProps(space.Props), PropImportSourceID) + if err := requireSameSpaceKey("space.props.import_source_id", spaceKey, manifest.Source.SpaceKey); err != nil { + return err + } + return nil +} + +// normalizePage validates and normalizes one page line into a StagedPage. +func normalizePage( + page *PageData, manifest *Manifest, lineNo, ordinal int, + seenPageIDs map[string]struct{}, parentOf map[string]string, + siblingCounter map[string]int, teamValues map[string]struct{}, res *InspectionResult, +) (*StagedPage, error) { + collectTeam(teamValues, stringOrEmpty(page.Team)) + + props := derefProps(page.Props) + externalID := propString(props, PropImportSourceID) + if externalID == "" { + return nil, inspectErr(InspectErrPageMissingID, "line %d: page is missing props.import_source_id", lineNo) + } + if _, dup := seenPageIDs[externalID]; dup { + return nil, inspectErr(InspectErrDuplicatePageID, "line %d: duplicate page external id %q", lineNo, externalID) + } + + title := strings.TrimSpace(stringOrEmpty(page.Title)) + if title == "" { + return nil, inspectErr(InspectErrPageMissingTitle, "line %d: page %q is missing a title", lineNo, externalID) + } + + // Space key must match the manifest. + if err := requireSameSpaceKey(fmt.Sprintf("page %q space_import_source_id", externalID), stringOrEmpty(page.SpaceImportSourceID), manifest.Source.SpaceKey); err != nil { + return nil, err + } + + parentID := stringOrEmpty(page.ParentImportSourceID) + if parentID != "" { + if _, seen := seenPageIDs[parentID]; !seen { + return nil, inspectErr(InspectErrParentNotSeen, "line %d: page %q references parent %q that has not appeared earlier", lineNo, externalID, parentID) + } + } + + // Decode and canonicalize the TipTap body independently of the line. + canonicalBody, searchText, links, tErr := CanonicalizeAndExtractSearchText(stringOrEmpty(page.Content)) + if tErr != nil { + return nil, inspectErr(InspectErrTipTap, "line %d: page %q content invalid: %v", lineNo, externalID, tErr) + } + + // Timestamp validation (does not discard the page). + sourceCreateAt := int64OrZero(page.CreateAt) + if !plausibleTimestamp(sourceCreateAt) { + res.Issues = append(res.Issues, InspectionIssue{ + Severity: SeverityWarning, Code: IssueSourceCreateAtInvalid, ExternalID: externalID, Title: title, + Message: "source create timestamp is missing, non-positive, or implausibly in the future", + Remediation: "The page is staged; execution falls back to the import time for CreateAt.", + Details: map[string]any{"source_create_at": sourceCreateAt}, + }) + } + // update_at issue is emitted only when supplied but unusable. + if page.UpdateAt != nil && !plausibleTimestamp(*page.UpdateAt) { + res.Issues = append(res.Issues, InspectionIssue{ + Severity: SeverityWarning, Code: IssueSourceUpdateAtInvalid, ExternalID: externalID, Title: title, + Message: "source update timestamp was supplied but is not usable", + Remediation: "The raw value is preserved in props; it does not affect local timestamps.", + Details: map[string]any{"source_update_at": *page.UpdateAt}, + }) + } + + sourceProps := allowlistSourceProps(props) + + incomingHash, hErr := HashSourceState(SourceStateHashInput{ + Title: title, + CanonicalBody: canonicalBody, + ParentExternalID: parentID, + AuthorAccountID: propString(props, PropConfluenceAuthorAccountID), + AuthorProposal: stringOrEmpty(page.User), + SourceCreateAt: sourceCreateAt, + SourceUpdateAt: int64OrZero(page.UpdateAt), + SourceProps: sourceProps, + }) + if hErr != nil { + return nil, inspectErr(InspectErrHash, "line %d: failed to hash page %q: %v", lineNo, externalID, hErr) + } + + // Count attachments and validate their paths (never opened). + if page.Attachments != nil { + for _, att := range *page.Attachments { + p := stringOrEmpty(att.Path) + if err := validateAttachmentPath(p, externalID, lineNo); err != nil { + return nil, err + } + res.AttachmentCount++ + } + if len(*page.Attachments) > 0 { + res.Issues = append(res.Issues, InspectionIssue{ + Severity: SeverityInfo, Code: IssueAttachmentNotImported, ExternalID: externalID, Title: title, + Message: fmt.Sprintf("%d attachment(s) counted but not imported in this release", len(*page.Attachments)), + Remediation: "Attachment import is a future release; bytes are neither extracted nor stored.", + }) + } + } + + // Report placeholders that appeared in ordinary text (never rewritten). + for _, l := range links { + if l.InText { + res.Issues = append(res.Issues, InspectionIssue{ + Severity: SeverityInfo, Code: IssuePlaceholderInText, ExternalID: externalID, Title: title, + Message: "a Confluence placeholder token appears in ordinary text and is left intact", + Remediation: "Placeholders in text are not rewritten in this release.", + }) + break + } + } + + seenPageIDs[externalID] = struct{}{} + parentOf[externalID] = parentID + sourceOrdinal := siblingCounter[parentID] + siblingCounter[parentID] = sourceOrdinal + 1 + + return &StagedPage{ + Ordinal: ordinal, + ExternalID: externalID, + ParentExternalID: parentID, + SourceOrdinal: sourceOrdinal, + Title: title, + CanonicalBody: canonicalBody, + SearchText: searchText, + SourceUserProposal: stringOrEmpty(page.User), + SourceAuthorAccountID: propString(props, PropConfluenceAuthorAccountID), + SourceCreateAt: sourceCreateAt, + SourceUpdateAt: int64OrZero(page.UpdateAt), + SourceProps: sourceProps, + IncomingSourceHash: incomingHash, + Links: links, + }, nil +} + +// allowlistSourceProps copies only the allowlisted source props, never arbitrary future producer +// fields. Returns a fresh map (nil-safe). +func allowlistSourceProps(props map[string]any) map[string]any { + out := make(map[string]any) + for _, key := range AllowlistedSourceProps { + if v, ok := props[key]; ok { + out[key] = v + } + } + return out +} + +// verifyHierarchy independently recomputes each page's depth from the parent map, rejecting cycles, +// missing parents, and depth greater than 10. It does not trust any producer flattening claim. +func verifyHierarchy(pages []StagedPage, parentOf map[string]string) error { + const maxDepth = 10 + for _, p := range pages { + depth := 0 + cur := p.ExternalID + for { + parent := parentOf[cur] + if parent == "" { + break + } + if _, ok := parentOf[parent]; !ok { + return inspectErr(InspectErrParentNotSeen, "page %q references missing parent %q", p.ExternalID, parent) + } + depth++ + if depth > maxDepth { + return inspectErr(InspectErrDepthExceeded, "page %q exceeds maximum hierarchy depth of %d", p.ExternalID, maxDepth) + } + if depth > len(parentOf) { + return inspectErr(InspectErrCycle, "page %q is part of a parent cycle", p.ExternalID) + } + cur = parent + } + } + return nil +} + +// requireSameSpaceKey rejects a non-empty space key that differs from the manifest's. An empty +// value is tolerated (some lines may omit it); only a present-and-different key is a mismatch. +func requireSameSpaceKey(where, value, manifestKey string) error { + if value == "" || manifestKey == "" { + return nil + } + if value != manifestKey { + return inspectErr(InspectErrSpaceKeyMismatch, "%s (%q) does not match manifest source space key (%q)", where, value, manifestKey) + } + return nil +} + +// validateAttachmentPath rejects an unsafe attachment path without opening the file. +func validateAttachmentPath(p, externalID string, lineNo int) error { + if p == "" { + return inspectErr(InspectErrAttachmentPath, "line %d: page %q has an attachment with an empty path", lineNo, externalID) + } + if strings.ContainsRune(p, 0) || strings.Contains(p, "\\") || strings.HasPrefix(p, "/") { + return inspectErr(InspectErrAttachmentPath, "line %d: page %q has an unsafe attachment path %q", lineNo, externalID, p) + } + for seg := range strings.SplitSeq(p, "/") { + if seg == "." || seg == ".." { + return inspectErr(InspectErrAttachmentPath, "line %d: page %q attachment path %q contains a %q segment", lineNo, externalID, p, seg) + } + } + return nil +} + +// reconcileCounts compares parsed counts against the manifest and warns on any mismatch. +func reconcileCounts(manifest *Manifest, res *InspectionResult) { + check := func(name string, parsed, declared int) { + if declared != parsed { + res.Issues = append(res.Issues, InspectionIssue{ + Severity: SeverityWarning, Code: IssueManifestCountMismatch, + Message: fmt.Sprintf("manifest declares %d %s but %d were parsed", declared, name, parsed), + Remediation: "The parsed counts are authoritative; the manifest may be stale.", + Details: map[string]any{"entity": name, "declared": declared, "parsed": parsed}, + }) + } + } + check("pages", len(res.Pages), manifest.Counts.Pages) + check("comments", res.CommentCount, manifest.Counts.Comments) + check("attachments", res.AttachmentCount, manifest.Counts.Attachments) +} + +// summarizeRestricted intersects the manifest restricted list with emitted (staged) page IDs. +func summarizeRestricted(manifest *Manifest, res *InspectionResult) { + staged := make(map[string]struct{}, len(res.Pages)) + for _, p := range res.Pages { + staged[p.ExternalID] = struct{}{} + } + seen := make(map[string]struct{}) + for _, rp := range manifest.RestrictedPages { + if _, dup := seen[rp.ID]; dup { + continue + } + seen[rp.ID] = struct{}{} + res.Restricted.ManifestTotal++ + if _, ok := staged[rp.ID]; ok { + res.Restricted.EmittedPages++ + res.Restricted.EmittedIDs = append(res.Restricted.EmittedIDs, rp.ID) + } else { + res.Restricted.ManifestOnly++ + res.Restricted.ManifestOnlyIDs = append(res.Restricted.ManifestOnlyIDs, rp.ID) + } + } +} + +// collectTeam records a non-empty advisory team value. +func collectTeam(teams map[string]struct{}, team string) { + if team != "" { + teams[team] = struct{}{} + } +} + +// emitTeamMismatch adds one aggregate warning when any advisory bundle team differs from the +// requested team name. It never reroutes. +func emitTeamMismatch(teams map[string]struct{}, opts InspectOptions, res *InspectionResult) { + if opts.RequestedTeamName == "" { + return + } + var mismatched []string + for t := range teams { + if t != opts.RequestedTeamName { + mismatched = append(mismatched, t) + } + } + if len(mismatched) > 0 { + res.Issues = append(res.Issues, InspectionIssue{ + Severity: SeverityWarning, Code: IssueBundleTeamMismatch, + Message: "advisory bundle team values differ from the requested target team", + Remediation: "The requested target team is authoritative; the bundle team is advisory only.", + Details: map[string]any{"requested_team": opts.RequestedTeamName, "bundle_teams": mismatched}, + }) + } +} + +// plausibleTimestamp reports whether ms is a positive epoch-millis value not implausibly far in the +// future (more than ~2 days ahead of a fixed sanity ceiling is rejected). It uses no wall clock so +// the pure function stays deterministic: any positive value up to year ~2100 is accepted. +func plausibleTimestamp(ms int64) bool { + if ms <= 0 { + return false + } + // Year 2100 in epoch millis; a source date beyond this is treated as implausible. + const year2100Millis = int64(4102444800000) + return ms <= year2100Millis +} diff --git a/server/importer/inspect_test.go b/server/importer/inspect_test.go new file mode 100644 index 0000000..fed36a2 --- /dev/null +++ b/server/importer/inspect_test.go @@ -0,0 +1,462 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package importer + +import ( + "archive/zip" + "bytes" + "errors" + "strings" + "testing" +) + +// inspectErrCode returns the stable code of an *InspectError or *ArchiveError, or "" otherwise. +func inspectErrCode(err error) string { + var ie *InspectError + if errors.As(err, &ie) { + return ie.Code + } + var ae *ArchiveError + if errors.As(err, &ae) { + return ae.Code + } + return "" +} + +func validBundle(t *testing.T) *bundleBuilder { + t.Helper() + jsonl := joinLines( + versionLine(), + spaceLine(), + pageLine(t, "100", "", "Home", docString("Welcome home")), + pageLine(t, "101", "100", "Child", docString("A child page")), + `{"type":"page_comment","page_comment":{"page_import_source_id":"100","user":"jdoe","content":"hi","create_at":1704625200000}}`, + resolveLine(), + ) + return newBundle(jsonl, baseManifest(2, 1, 0)) +} + +func TestInspect_ValidBundle(t *testing.T) { + res, err := validBundle(t).inspect(t, InspectOptions{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if res.Version != 2 { + t.Errorf("version = %d, want 2", res.Version) + } + if len(res.Pages) != 2 { + t.Fatalf("pages = %d, want 2", len(res.Pages)) + } + if res.CommentCount != 1 { + t.Errorf("comments = %d, want 1", res.CommentCount) + } + if res.SpaceKey != "DOCS" { + t.Errorf("space key = %q, want DOCS", res.SpaceKey) + } + if res.SpaceTitle != "Docs" { + t.Errorf("space title = %q, want Docs", res.SpaceTitle) + } + // Root then child; child's parent + sibling ordinals. + if res.Pages[0].ExternalID != "100" || res.Pages[1].ExternalID != "101" { + t.Errorf("page order = %q, %q", res.Pages[0].ExternalID, res.Pages[1].ExternalID) + } + if res.Pages[1].ParentExternalID != "100" { + t.Errorf("child parent = %q, want 100", res.Pages[1].ParentExternalID) + } + if res.Pages[0].IncomingSourceHash == "" || !IsValidSHA256Hex(res.Pages[0].IncomingSourceHash) { + t.Errorf("incoming hash invalid: %q", res.Pages[0].IncomingSourceHash) + } +} + +func TestInspect_ChecksumMismatch(t *testing.T) { + b := validBundle(t) + b.manifest.Checksums.JSONLSha256 = strings.Repeat("a", 64) + b.skipChecksum = true + _, err := b.inspect(t, InspectOptions{}) + if got := inspectErrCode(err); got != InspectErrChecksumMismatch { + t.Fatalf("code = %q, want %q", got, InspectErrChecksumMismatch) + } +} + +func TestInspect_MissingChecksum(t *testing.T) { + b := validBundle(t) + b.skipChecksum = true // leave checksum empty + _, err := b.inspect(t, InspectOptions{}) + if got := inspectErrCode(err); got != InspectErrChecksumMissing { + t.Fatalf("code = %q, want %q", got, InspectErrChecksumMissing) + } +} + +func TestInspect_ManifestReportsErrors(t *testing.T) { + b := validBundle(t) + b.manifest.Errors = []string{"conversion failed"} + _, err := b.inspect(t, InspectOptions{}) + if got := inspectErrCode(err); got != InspectErrManifestHasErrors { + t.Fatalf("code = %q, want %q", got, InspectErrManifestHasErrors) + } +} + +func TestInspect_WrongManifestVersion(t *testing.T) { + b := validBundle(t) + b.manifest.Version = "1" + _, err := b.inspect(t, InspectOptions{}) + if got := inspectErrCode(err); got != InspectErrManifestVersion { + t.Fatalf("code = %q, want %q", got, InspectErrManifestVersion) + } +} + +func TestInspect_WrongVersionLine(t *testing.T) { + jsonl := joinLines( + `{"type":"version","version":3,"source":{"space_key":"DOCS"}}`, + spaceLine(), + resolveLine(), + ) + _, err := newBundle(jsonl, baseManifest(0, 0, 0)).inspect(t, InspectOptions{}) + if got := inspectErrCode(err); got != InspectErrVersionValue { + t.Fatalf("code = %q, want %q", got, InspectErrVersionValue) + } +} + +func TestInspect_BadSequence_PageBeforeSpace(t *testing.T) { + jsonl := joinLines( + versionLine(), + pageLine(t, "100", "", "Home", docString("x")), + spaceLine(), + resolveLine(), + ) + _, err := newBundle(jsonl, baseManifest(1, 0, 0)).inspect(t, InspectOptions{}) + if got := inspectErrCode(err); got != InspectErrSequence { + t.Fatalf("code = %q, want %q", got, InspectErrSequence) + } +} + +func TestInspect_MissingResolveLine(t *testing.T) { + jsonl := joinLines(versionLine(), spaceLine(), pageLine(t, "100", "", "Home", docString("x"))) + _, err := newBundle(jsonl, baseManifest(1, 0, 0)).inspect(t, InspectOptions{}) + if got := inspectErrCode(err); got != InspectErrSequence { + t.Fatalf("code = %q, want %q", got, InspectErrSequence) + } +} + +func TestInspect_UnknownType(t *testing.T) { + jsonl := joinLines(versionLine(), spaceLine(), `{"type":"widget"}`, resolveLine()) + _, err := newBundle(jsonl, baseManifest(0, 0, 0)).inspect(t, InspectOptions{}) + if got := inspectErrCode(err); got != InspectErrUnknownType { + t.Fatalf("code = %q, want %q", got, InspectErrUnknownType) + } +} + +func TestInspect_BlankLine(t *testing.T) { + jsonl := versionLine() + "\n" + spaceLine() + "\n\n" + resolveLine() + "\n" + _, err := newBundle(jsonl, baseManifest(0, 0, 0)).inspect(t, InspectOptions{}) + if got := inspectErrCode(err); got != InspectErrBlankLine { + t.Fatalf("code = %q, want %q", got, InspectErrBlankLine) + } +} + +func TestInspect_TrailingLineAfterResolve(t *testing.T) { + jsonl := joinLines(versionLine(), spaceLine(), resolveLine(), spaceLine()) + _, err := newBundle(jsonl, baseManifest(0, 0, 0)).inspect(t, InspectOptions{}) + if got := inspectErrCode(err); got != InspectErrSequence { + t.Fatalf("code = %q, want %q", got, InspectErrSequence) + } +} + +func TestInspect_DuplicatePageID(t *testing.T) { + jsonl := joinLines( + versionLine(), spaceLine(), + pageLine(t, "100", "", "A", docString("a")), + pageLine(t, "100", "", "B", docString("b")), + resolveLine(), + ) + _, err := newBundle(jsonl, baseManifest(2, 0, 0)).inspect(t, InspectOptions{}) + if got := inspectErrCode(err); got != InspectErrDuplicatePageID { + t.Fatalf("code = %q, want %q", got, InspectErrDuplicatePageID) + } +} + +func TestInspect_ChildBeforeParent(t *testing.T) { + jsonl := joinLines( + versionLine(), spaceLine(), + pageLine(t, "101", "100", "Child", docString("c")), + pageLine(t, "100", "", "Parent", docString("p")), + resolveLine(), + ) + _, err := newBundle(jsonl, baseManifest(2, 0, 0)).inspect(t, InspectOptions{}) + if got := inspectErrCode(err); got != InspectErrParentNotSeen { + t.Fatalf("code = %q, want %q", got, InspectErrParentNotSeen) + } +} + +func TestInspect_MissingParent(t *testing.T) { + jsonl := joinLines( + versionLine(), spaceLine(), + pageLine(t, "100", "999", "Orphan", docString("o")), + resolveLine(), + ) + _, err := newBundle(jsonl, baseManifest(1, 0, 0)).inspect(t, InspectOptions{}) + if got := inspectErrCode(err); got != InspectErrParentNotSeen { + t.Fatalf("code = %q, want %q", got, InspectErrParentNotSeen) + } +} + +func TestInspect_DepthExceeded(t *testing.T) { + // Build a chain of 12 pages (depth 11 at the deepest) which must be rejected. + lines := []string{versionLine(), spaceLine()} + prev := "" + for i := 0; i <= 11; i++ { + id := string(rune('a'+i)) + "id" + lines = append(lines, pageLine(t, id, prev, "P", docString("x"))) + prev = id + } + lines = append(lines, resolveLine()) + jsonl := joinLines(lines...) + _, err := newBundle(jsonl, baseManifest(12, 0, 0)).inspect(t, InspectOptions{}) + if got := inspectErrCode(err); got != InspectErrDepthExceeded { + t.Fatalf("code = %q, want %q", got, InspectErrDepthExceeded) + } +} + +func TestInspect_DepthTenAllowed(t *testing.T) { + // A chain of 11 pages has a maximum depth of 10, which is allowed. + lines := []string{versionLine(), spaceLine()} + prev := "" + for i := 0; i <= 10; i++ { + id := string(rune('a'+i)) + "id" + lines = append(lines, pageLine(t, id, prev, "P", docString("x"))) + prev = id + } + lines = append(lines, resolveLine()) + jsonl := joinLines(lines...) + _, err := newBundle(jsonl, baseManifest(11, 0, 0)).inspect(t, InspectOptions{}) + if err != nil { + t.Fatalf("depth 10 should be allowed, got %v", err) + } +} + +func TestInspect_CountsAndAttachments(t *testing.T) { + pageWithAtt := `{"type":"page","page":{"space_import_source_id":"DOCS","user":"j","title":"WithAtt","content":` + + mustQuote(docString("x")) + + `,"props":{"import_source_id":"200"},"attachments":[{"path":"200/a.png"},{"path":"200/b.png"}]}}` + jsonl := joinLines(versionLine(), spaceLine(), pageWithAtt, resolveLine()) + res, err := newBundle(jsonl, baseManifest(1, 0, 2)).inspect(t, InspectOptions{}) + if err != nil { + t.Fatalf("unexpected: %v", err) + } + if res.AttachmentCount != 2 { + t.Errorf("attachments = %d, want 2", res.AttachmentCount) + } + if !hasIssue(res, IssueAttachmentNotImported) { + t.Errorf("expected attachment_placeholder_not_imported issue") + } +} + +func TestInspect_ManifestCountMismatch(t *testing.T) { + res, err := newBundle( + joinLines(versionLine(), spaceLine(), pageLine(t, "100", "", "H", docString("x")), resolveLine()), + baseManifest(5, 0, 0), // declares 5 pages, but only 1 parsed + ).inspect(t, InspectOptions{}) + if err != nil { + t.Fatalf("unexpected: %v", err) + } + if !hasIssue(res, IssueManifestCountMismatch) { + t.Errorf("expected manifest_count_mismatch issue") + } +} + +func TestInspect_RestrictedPages(t *testing.T) { + b := newBundle( + joinLines(versionLine(), spaceLine(), + pageLine(t, "100", "", "H", docString("x")), + pageLine(t, "101", "100", "C", docString("y")), + resolveLine()), + baseManifest(2, 0, 0), + ) + b.manifest.RestrictedPages = []ManifestRestrictedPage{ + {ID: "100", Title: "H"}, // emitted + {ID: "999", Title: "Gone"}, // manifest-only + } + res, err := b.inspect(t, InspectOptions{}) + if err != nil { + t.Fatalf("unexpected: %v", err) + } + if res.Restricted.ManifestTotal != 2 || res.Restricted.EmittedPages != 1 || res.Restricted.ManifestOnly != 1 { + t.Errorf("restricted summary = %+v", res.Restricted) + } +} + +func TestInspect_TeamMismatch(t *testing.T) { + res, err := validBundle(t).inspect(t, InspectOptions{RequestedTeamName: "other-team"}) + if err != nil { + t.Fatalf("unexpected: %v", err) + } + if !hasIssue(res, IssueBundleTeamMismatch) { + t.Errorf("expected bundle_team_mismatch issue") + } +} + +func TestInspect_TeamMatchNoIssue(t *testing.T) { + res, err := validBundle(t).inspect(t, InspectOptions{RequestedTeamName: "myteam"}) + if err != nil { + t.Fatalf("unexpected: %v", err) + } + if hasIssue(res, IssueBundleTeamMismatch) { + t.Errorf("did not expect bundle_team_mismatch issue") + } +} + +func TestInspect_SpaceKeyMismatch(t *testing.T) { + // A page declaring a different space key must be rejected. + badPage := `{"type":"page","page":{"space_import_source_id":"OTHER","user":"j","title":"H","content":` + + mustQuote(docString("x")) + `,"props":{"import_source_id":"100"}}}` + jsonl := joinLines(versionLine(), spaceLine(), badPage, resolveLine()) + _, err := newBundle(jsonl, baseManifest(1, 0, 0)).inspect(t, InspectOptions{}) + if got := inspectErrCode(err); got != InspectErrSpaceKeyMismatch { + t.Fatalf("code = %q, want %q", got, InspectErrSpaceKeyMismatch) + } +} + +func TestInspect_InvalidTipTap(t *testing.T) { + jsonl := joinLines(versionLine(), spaceLine(), + pageLine(t, "100", "", "H", `{"type":"notdoc"}`), resolveLine()) + _, err := newBundle(jsonl, baseManifest(1, 0, 0)).inspect(t, InspectOptions{}) + if got := inspectErrCode(err); got != InspectErrTipTap { + t.Fatalf("code = %q, want %q", got, InspectErrTipTap) + } +} + +func TestInspect_InvalidCreateAtTimestamp(t *testing.T) { + page := `{"type":"page","page":{"space_import_source_id":"DOCS","user":"j","title":"H","content":` + + mustQuote(docString("x")) + `,"create_at":-5,"props":{"import_source_id":"100"}}}` + jsonl := joinLines(versionLine(), spaceLine(), page, resolveLine()) + res, err := newBundle(jsonl, baseManifest(1, 0, 0)).inspect(t, InspectOptions{}) + if err != nil { + t.Fatalf("unexpected: %v", err) + } + if !hasIssue(res, IssueSourceCreateAtInvalid) { + t.Errorf("expected source_create_at_invalid issue") + } +} + +// --- archive-level tests --- + +func TestInspectArchive_MissingJSONL(t *testing.T) { + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + w, _ := zw.Create(entryManifest) + _, _ = w.Write([]byte(`{"version":"2"}`)) + _ = zw.Close() + raw := buf.Bytes() + _, err := InspectArchive(bytes.NewReader(raw), int64(len(raw))) + if got := inspectErrCode(err); got != ArchiveErrMissingJSONL { + t.Fatalf("code = %q, want %q", got, ArchiveErrMissingJSONL) + } +} + +func TestInspectArchive_MissingManifest(t *testing.T) { + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + w, _ := zw.Create(entryJSONL) + _, _ = w.Write([]byte("x")) + _ = zw.Close() + raw := buf.Bytes() + _, err := InspectArchive(bytes.NewReader(raw), int64(len(raw))) + if got := inspectErrCode(err); got != ArchiveErrMissingManifest { + t.Fatalf("code = %q, want %q", got, ArchiveErrMissingManifest) + } +} + +func TestInspectArchive_UnexpectedEntry(t *testing.T) { + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + for _, n := range []string{entryManifest, entryJSONL, "notes.txt"} { + w, _ := zw.Create(n) + _, _ = w.Write([]byte("x")) + } + _ = zw.Close() + raw := buf.Bytes() + _, err := InspectArchive(bytes.NewReader(raw), int64(len(raw))) + if got := inspectErrCode(err); got != ArchiveErrUnexpectedEntry { + t.Fatalf("code = %q, want %q", got, ArchiveErrUnexpectedEntry) + } +} + +func TestInspectArchive_DataDirAllowedNotOpened(t *testing.T) { + b := validBundle(t) + b.withFile("data/100/diagram.png", "not-real-bytes") + res, err := b.inspect(t, InspectOptions{}) + if err != nil { + t.Fatalf("data/ should be allowed: %v", err) + } + if res == nil { + t.Fatal("nil result") + } +} + +func TestInspectArchive_Traversal(t *testing.T) { + cases := map[string]string{ + "absolute": "/etc/passwd", + "dotdot": "../evil", + "backslash": "a\\b", + "drive": "C:evil", + } + for name, entry := range cases { + t.Run(name, func(t *testing.T) { + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + for _, n := range []string{entryManifest, entryJSONL} { + w, _ := zw.Create(n) + _, _ = w.Write([]byte("x")) + } + // zip.Writer.Create sanitizes some names; CreateHeader writes the raw name verbatim. + hw, err := zw.CreateHeader(&zip.FileHeader{Name: entry, Method: zip.Store}) + if err != nil { + t.Fatalf("zip writer refused name %q: %v", entry, err) + } + _, _ = hw.Write([]byte("x")) + _ = zw.Close() + raw := buf.Bytes() + _, err = InspectArchive(bytes.NewReader(raw), int64(len(raw))) + if err == nil { + t.Fatalf("expected rejection of entry %q", entry) + } + code := inspectErrCode(err) + if code != ArchiveErrUnsafeEntry && code != ArchiveErrBadEntryName { + t.Fatalf("code = %q, want unsafe/bad-name for entry %q", code, entry) + } + }) + } +} + +func TestInspectArchive_DuplicateEntry(t *testing.T) { + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + for _, n := range []string{entryManifest, entryJSONL, entryJSONL} { + w, _ := zw.Create(n) + _, _ = w.Write([]byte("x")) + } + _ = zw.Close() + raw := buf.Bytes() + _, err := InspectArchive(bytes.NewReader(raw), int64(len(raw))) + if got := inspectErrCode(err); got != ArchiveErrDuplicateEntry { + t.Fatalf("code = %q, want %q", got, ArchiveErrDuplicateEntry) + } +} + +// helpers + +func hasIssue(res *InspectionResult, code string) bool { + for _, i := range res.Issues { + if i.Code == code { + return true + } + } + return false +} + +// mustQuote JSON-encodes s as a JSON string literal (including surrounding quotes). +func mustQuote(s string) string { + b, _ := jsonMarshalString(s) + return b +} diff --git a/server/importer/links.go b/server/importer/links.go new file mode 100644 index 0000000..dab1c2d --- /dev/null +++ b/server/importer/links.go @@ -0,0 +1,77 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package importer + +import "strings" + +// Confluence link placeholder prefixes the producer emits inside TipTap link mark hrefs and image +// src attributes. V1 discovers these structurally but never rewrites them: there is no canonical +// Docs reader URL in this repository yet. +const ( + PlaceholderPageID = "CONF_PAGE_ID" + PlaceholderPageTitle = "CONF_PAGE_TITLE" + PlaceholderFile = "CONF_FILE" + PlaceholderAttachment = "CONF_ATTACHMENT" +) + +// LinkKind classifies a discovered placeholder. +type LinkKind string + +const ( + LinkKindPageID LinkKind = "page_id" + LinkKindPageTitle LinkKind = "page_title" + LinkKindFile LinkKind = "file" + LinkKindAttachment LinkKind = "attachment" +) + +// DiscoveredLink is one placeholder found in an approved attribute (link mark href or image src). +// Raw is the exact attribute value; Target is the portion after the placeholder prefix and colon +// (e.g. the page ID or title), when present. +type DiscoveredLink struct { + Kind LinkKind `json:"kind"` + Raw string `json:"raw"` + Target string `json:"target"` + // InImageSrc is true when the placeholder was found in an image node's src rather than a link + // mark's href. + InImageSrc bool `json:"in_image_src"` + // InText is true when a placeholder token appeared in ordinary text rather than an approved + // attribute. V1 never rewrites these; the caller reports them as placeholder_in_text_not_rewritten. + InText bool `json:"in_text"` +} + +// classifyPlaceholder inspects an attribute value and returns a DiscoveredLink when it begins with +// a recognized placeholder prefix, plus ok=true. A value that merely contains a placeholder token +// somewhere other than the start is not treated as a placeholder here (ordinary text is reported +// separately by the caller). +func classifyPlaceholder(value string, inImageSrc bool) (DiscoveredLink, bool) { + for prefix, kind := range placeholderKinds { + if value == prefix || strings.HasPrefix(value, prefix+":") { + target := "" + if len(value) > len(prefix)+1 { + target = value[len(prefix)+1:] + } + return DiscoveredLink{Kind: kind, Raw: value, Target: target, InImageSrc: inImageSrc}, true + } + } + return DiscoveredLink{}, false +} + +var placeholderKinds = map[string]LinkKind{ + PlaceholderPageID: LinkKindPageID, + PlaceholderPageTitle: LinkKindPageTitle, + PlaceholderFile: LinkKindFile, + PlaceholderAttachment: LinkKindAttachment, +} + +// containsPlaceholderToken reports whether s contains any placeholder prefix token anywhere. Used +// to flag placeholders that appear in ordinary text (which V1 never rewrites) so the report can +// note them. +func containsPlaceholderToken(s string) bool { + for prefix := range placeholderKinds { + if strings.Contains(s, prefix) { + return true + } + } + return false +} diff --git a/server/importer/testhelpers_test.go b/server/importer/testhelpers_test.go new file mode 100644 index 0000000..ca48f63 --- /dev/null +++ b/server/importer/testhelpers_test.go @@ -0,0 +1,170 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package importer + +import ( + "archive/zip" + "bytes" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "strings" + "testing" +) + +// bundleBuilder assembles an in-memory ZIP bundle for tests. It computes the JSONL checksum and +// injects it into the manifest unless the manifest already carries one. +type bundleBuilder struct { + jsonl string + manifest Manifest + extraFiles map[string]string // extra archive entries (name -> body), e.g. under data/ + // skipChecksum leaves the manifest checksum untouched (for checksum-mismatch tests). + skipChecksum bool +} + +func newBundle(jsonl string, manifest Manifest) *bundleBuilder { + return &bundleBuilder{jsonl: jsonl, manifest: manifest, extraFiles: map[string]string{}} +} + +func (b *bundleBuilder) withFile(name, body string) *bundleBuilder { + b.extraFiles[name] = body + return b +} + +// jsonlSha returns the lowercase-hex SHA-256 of the builder's JSONL bytes. +func (b *bundleBuilder) jsonlSha() string { + sum := sha256.Sum256([]byte(b.jsonl)) + return hex.EncodeToString(sum[:]) +} + +// bytes builds the ZIP archive bytes. +func (b *bundleBuilder) bytesZip(t *testing.T) []byte { + t.Helper() + m := b.manifest + if !b.skipChecksum && m.Checksums.JSONLSha256 == "" { + m.Checksums.JSONLSha256 = b.jsonlSha() + } + manifestBytes, err := json.Marshal(m) + if err != nil { + t.Fatalf("marshal manifest: %v", err) + } + + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + writeEntry := func(name, body string) { + w, err := zw.Create(name) + if err != nil { + t.Fatalf("zip create %q: %v", name, err) + } + if _, err := w.Write([]byte(body)); err != nil { + t.Fatalf("zip write %q: %v", name, err) + } + } + writeEntry(entryManifest, string(manifestBytes)) + writeEntry(entryJSONL, b.jsonl) + for name, body := range b.extraFiles { + writeEntry(name, body) + } + if err := zw.Close(); err != nil { + t.Fatalf("zip close: %v", err) + } + return buf.Bytes() +} + +// inspectBundle runs InspectArchive + Inspect over the builder's bytes. +func (b *bundleBuilder) inspect(t *testing.T, opts InspectOptions) (*InspectionResult, error) { + t.Helper() + raw := b.bytesZip(t) + contents, err := InspectArchive(bytes.NewReader(raw), int64(len(raw))) + if err != nil { + return nil, err + } + return Inspect(contents, opts) +} + +// baseManifest returns a minimal valid v2 manifest with the given counts. +func baseManifest(pages, comments, attachments int) Manifest { + return Manifest{ + Version: "2", + Source: ManifestSource{Type: "confluence", SpaceKey: "DOCS", SpaceName: "Docs"}, + Target: ManifestTarget{Team: "myteam"}, + Counts: ManifestCounts{Pages: pages, Comments: comments, Attachments: attachments}, + } +} + +// docString builds a TipTap doc JSON string with a single paragraph of the given text. +func docString(text string) string { + doc := map[string]any{ + "type": "doc", + "content": []any{ + map[string]any{ + "type": "paragraph", + "content": []any{ + map[string]any{"type": "text", "text": text}, + }, + }, + }, + } + b, _ := json.Marshal(doc) + return string(b) +} + +// versionLine, spaceLine, pageLine, commentLine, resolveLine build individual JSONL lines. +func versionLine() string { + return `{"type":"version","version":2,"source":{"space_key":"DOCS"}}` +} + +func spaceLine() string { + return `{"type":"space","space":{"team":"myteam","title":"Docs","description":"Migrated","props":{"import_source_id":"DOCS"}}}` +} + +// pageLine builds a page line. parentID "" means a root page. content is a raw TipTap JSON string. +func pageLine(t *testing.T, externalID, parentID, title, content string) string { + t.Helper() + page := map[string]any{ + "type": "page", + "page": map[string]any{ + "team": "myteam", + "space_import_source_id": "DOCS", + "user": "jdoe", + "title": title, + "content": content, + "create_at": int64(1704106800000), + "update_at": int64(1704193200000), + "props": map[string]any{ + "import_source_id": externalID, + "import_source": "confluence", + "confluence_author_account_id": "aaid-" + externalID, + }, + }, + } + if parentID != "" { + page["page"].(map[string]any)["parent_import_source_id"] = parentID + } + b, err := json.Marshal(page) + if err != nil { + t.Fatalf("marshal page line: %v", err) + } + return string(b) +} + +func resolveLine() string { + return `{"type":"resolve_space_placeholders","resolve_space_placeholders":{"team":"myteam","space_import_source_id":"DOCS"}}` +} + +// jsonMarshalString JSON-encodes s as a quoted JSON string literal. +func jsonMarshalString(s string) (string, error) { + b, err := json.Marshal(s) + return string(b), err +} + +// joinLines joins JSONL lines with newlines and a trailing newline (a normal file terminator). +func joinLines(lines ...string) string { + var b strings.Builder + for _, l := range lines { + b.WriteString(l) + b.WriteByte('\n') + } + return b.String() +} diff --git a/server/importer/tiptap.go b/server/importer/tiptap.go new file mode 100644 index 0000000..508683f --- /dev/null +++ b/server/importer/tiptap.go @@ -0,0 +1,253 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package importer + +import ( + "bytes" + "encoding/json" + "fmt" + "regexp" + "strings" + + "github.com/mattermost/mattermost-plugin-docs/server/model" +) + +// TipTapError describes a rejected TipTap document with a stable code. +type TipTapError struct { + Code string + Message string +} + +func (e *TipTapError) Error() string { return e.Message } + +func tiptapErr(code, format string, args ...any) *TipTapError { + return &TipTapError{Code: code, Message: fmt.Sprintf(format, args...)} +} + +// Stable TipTap rejection codes. +const ( + TipTapErrInvalidJSON = "tiptap_invalid_json" + TipTapErrNotDoc = "tiptap_not_doc" + TipTapErrBadContent = "tiptap_bad_content" + TipTapErrBadMarks = "tiptap_bad_marks" + TipTapErrBadText = "tiptap_bad_text" + TipTapErrTooManyNodes = "tiptap_too_many_nodes" + TipTapErrTooDeep = "tiptap_too_deep" + TipTapErrBodyTooLarge = "tiptap_body_too_large" + TipTapErrSearchTooLarge = "tiptap_search_text_too_large" +) + +// Node/mark type names with SearchText significance. +const ( + nodeTypeDoc = "doc" + nodeTypeText = "text" + nodeTypeHardBreak = "hardBreak" + nodeTypeImage = "image" + markTypeLink = "link" +) + +// blockSeparatorTypes are node types after which SearchText emits a newline separator so distinct +// blocks do not run together. +var blockSeparatorTypes = map[string]struct{}{ + "paragraph": {}, + "heading": {}, + "listItem": {}, + "codeBlock": {}, + "blockquote": {}, + "tableCell": {}, + "tableHeader": {}, + "tableRow": {}, +} + +// CanonicalizeAndExtractSearchText validates a TipTap document (a JSON string), returning its +// compact canonical re-marshaling, the derived plain-text SearchText, and any placeholder links +// discovered in approved attributes. Unknown node/mark types and attributes are preserved. +// +// The canonical body is a deterministic compact re-marshaling (Go sorts object keys), so it is +// stable for hashing regardless of the producer's original key order. +func CanonicalizeAndExtractSearchText(body string) (canonicalBody string, searchText string, links []DiscoveredLink, err error) { + dec := json.NewDecoder(strings.NewReader(body)) + dec.UseNumber() + + var root any + if decErr := dec.Decode(&root); decErr != nil { + return "", "", nil, tiptapErr(TipTapErrInvalidJSON, "content is not valid JSON: %v", decErr) + } + // Reject trailing data after the first JSON value. + if dec.More() { + return "", "", nil, tiptapErr(TipTapErrInvalidJSON, "content has trailing data after the root JSON value") + } + + rootObj, ok := root.(map[string]any) + if !ok { + return "", "", nil, tiptapErr(TipTapErrNotDoc, "content root is not a JSON object") + } + if t, _ := rootObj["type"].(string); t != nodeTypeDoc { + return "", "", nil, tiptapErr(TipTapErrNotDoc, "content root type is not %q", nodeTypeDoc) + } + + w := &tiptapWalker{} + if walkErr := w.walkNode(rootObj, 0); walkErr != nil { + return "", "", nil, walkErr + } + + compact, marshalErr := json.Marshal(rootObj) + if marshalErr != nil { + return "", "", nil, tiptapErr(TipTapErrInvalidJSON, "failed to re-marshal content: %v", marshalErr) + } + if len(compact) > model.PageBodyMaxBytes { + return "", "", nil, tiptapErr(TipTapErrBodyTooLarge, "canonical body is %d bytes, limit is %d", len(compact), model.PageBodyMaxBytes) + } + + st := normalizeSearchText(w.search.String()) + if len(st) > model.PageSearchTextMaxBytes { + return "", "", nil, tiptapErr(TipTapErrSearchTooLarge, "search text is %d bytes, limit is %d", len(st), model.PageSearchTextMaxBytes) + } + + return string(compact), st, w.links, nil +} + +// tiptapWalker accumulates SearchText and discovered links across a depth-first traversal, while +// enforcing node-count and depth limits. +type tiptapWalker struct { + search strings.Builder + links []DiscoveredLink + nodeCount int +} + +// walkNode processes one node object at the given nesting depth (root doc is depth 0). +func (w *tiptapWalker) walkNode(node map[string]any, depth int) error { + w.nodeCount++ + if w.nodeCount > MaxTipTapNodes { + return tiptapErr(TipTapErrTooManyNodes, "document has more than %d nodes", MaxTipTapNodes) + } + if depth > MaxTipTapDepth { + return tiptapErr(TipTapErrTooDeep, "document nesting exceeds depth %d", MaxTipTapDepth) + } + + nodeType, _ := node["type"].(string) + + // Marks (if present) must be an array; scan link marks for placeholder hrefs. + if rawMarks, present := node["marks"]; present { + marks, ok := rawMarks.([]any) + if !ok { + return tiptapErr(TipTapErrBadMarks, "node %q has a non-array marks field", nodeType) + } + for _, rm := range marks { + mark, ok := rm.(map[string]any) + if !ok { + return tiptapErr(TipTapErrBadMarks, "node %q has a non-object mark", nodeType) + } + w.scanMark(mark) + } + } + + // Image nodes: scan attrs.src. + if nodeType == nodeTypeImage { + if src := attrString(node, "src"); src != "" { + if link, ok := classifyPlaceholder(src, true); ok { + w.links = append(w.links, link) + } + } + } + + switch nodeType { + case nodeTypeText: + rawText, present := node["text"] + if present { + text, ok := rawText.(string) + if !ok { + return tiptapErr(TipTapErrBadText, "text node has a non-string text field") + } + w.search.WriteString(text) + if containsPlaceholderToken(text) { + w.links = append(w.links, DiscoveredLink{Raw: text, InText: true}) + } + } + case nodeTypeHardBreak: + w.search.WriteByte('\n') + } + + // Recurse into content (if present, must be an array). + if rawContent, present := node["content"]; present { + content, ok := rawContent.([]any) + if !ok { + return tiptapErr(TipTapErrBadContent, "node %q has a non-array content field", nodeType) + } + for _, rc := range content { + child, ok := rc.(map[string]any) + if !ok { + return tiptapErr(TipTapErrBadContent, "node %q has a non-object content child", nodeType) + } + if err := w.walkNode(child, depth+1); err != nil { + return err + } + } + } + + // Emit a block separator after block-level nodes so their text does not merge with the next. + if _, isBlock := blockSeparatorTypes[nodeType]; isBlock { + w.search.WriteByte('\n') + } + + return nil +} + +// scanMark records a placeholder discovered in a link mark's href attribute. +func (w *tiptapWalker) scanMark(mark map[string]any) { + if t, _ := mark["type"].(string); t != markTypeLink { + return + } + href := attrString(mark, "href") + if href == "" { + return + } + if link, ok := classifyPlaceholder(href, false); ok { + w.links = append(w.links, link) + } +} + +// attrString reads a string attribute from an object's "attrs" map, returning "" if absent or of a +// non-string type. +func attrString(obj map[string]any, key string) string { + attrs, ok := obj["attrs"].(map[string]any) + if !ok { + return "" + } + v, _ := attrs[key].(string) + return v +} + +var ( + // horizontalWhitespace matches runs of spaces/tabs to collapse to a single space. + horizontalWhitespace = regexp.MustCompile(`[ \t]+`) + // threePlusNewlines matches runs of 3+ newlines (allowing surrounding horizontal space) to + // collapse to exactly two. + threePlusNewlines = regexp.MustCompile(`\n[ \t]*(\n[ \t]*){2,}`) +) + +// normalizeSearchText collapses horizontal whitespace and excess blank lines, then trims. +func normalizeSearchText(s string) string { + // Normalize CRLF/CR to LF first so newline collapsing is uniform. + s = strings.ReplaceAll(s, "\r\n", "\n") + s = strings.ReplaceAll(s, "\r", "\n") + s = horizontalWhitespace.ReplaceAllString(s, " ") + // Trim trailing horizontal space on each line so " \n" does not defeat newline collapsing. + s = trimLineTrailingSpaces(s) + s = threePlusNewlines.ReplaceAllString(s, "\n\n") + return strings.TrimSpace(s) +} + +// trimLineTrailingSpaces removes trailing spaces/tabs before each newline. +func trimLineTrailingSpaces(s string) string { + var b bytes.Buffer + lines := strings.Split(s, "\n") + for i, line := range lines { + b.WriteString(strings.TrimRight(line, " \t")) + if i < len(lines)-1 { + b.WriteByte('\n') + } + } + return b.String() +} diff --git a/server/importer/tiptap_test.go b/server/importer/tiptap_test.go new file mode 100644 index 0000000..22dc60c --- /dev/null +++ b/server/importer/tiptap_test.go @@ -0,0 +1,182 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package importer + +import ( + "encoding/json" + "strings" + "testing" +) + +func TestCanonicalize_RejectsNonDoc(t *testing.T) { + for _, body := range []string{`{"type":"paragraph"}`, `[]`, `"text"`, `{`, `{"type":"doc"} trailing`} { + if _, _, _, err := CanonicalizeAndExtractSearchText(body); err == nil { + t.Errorf("expected rejection for %q", body) + } + } +} + +func TestCanonicalize_PreservesUnknownTypes(t *testing.T) { + body := `{"type":"doc","content":[{"type":"customWidget","attrs":{"foo":"bar"},"content":[{"type":"text","text":"hi"}]}]}` + canon, search, _, err := CanonicalizeAndExtractSearchText(body) + if err != nil { + t.Fatalf("unexpected: %v", err) + } + if !strings.Contains(canon, "customWidget") || !strings.Contains(canon, "bar") { + t.Errorf("unknown node/attr not preserved: %s", canon) + } + if search != "hi" { + t.Errorf("search = %q, want hi", search) + } +} + +func TestCanonicalize_RejectsNonArrayContent(t *testing.T) { + if _, _, _, err := CanonicalizeAndExtractSearchText(`{"type":"doc","content":{}}`); err == nil { + t.Errorf("expected bad-content rejection") + } + if _, _, _, err := CanonicalizeAndExtractSearchText(`{"type":"doc","content":[{"type":"text","text":5}]}`); err == nil { + t.Errorf("expected bad-text rejection") + } +} + +func TestSearchText_ParagraphsHeadingsHardBreak(t *testing.T) { + doc := map[string]any{ + "type": "doc", + "content": []any{ + block("heading", text("Title")), + block("paragraph", text("First line"), map[string]any{"type": "hardBreak"}, text("second line")), + block("paragraph", text("Another paragraph")), + }, + } + _, search, _, err := CanonicalizeAndExtractSearchText(marshal(doc)) + if err != nil { + t.Fatalf("unexpected: %v", err) + } + want := "Title\nFirst line\nsecond line\nAnother paragraph" + if search != want { + t.Errorf("search = %q, want %q", search, want) + } +} + +func TestSearchText_ListsAndTables(t *testing.T) { + doc := map[string]any{ + "type": "doc", + "content": []any{ + block("bulletList", + block("listItem", block("paragraph", text("one"))), + block("listItem", block("paragraph", text("two"))), + ), + block("table", + block("tableRow", + block("tableHeader", block("paragraph", text("H1"))), + block("tableHeader", block("paragraph", text("H2"))), + ), + block("tableRow", + block("tableCell", block("paragraph", text("a"))), + block("tableCell", block("paragraph", text("b"))), + ), + ), + }, + } + _, search, _, err := CanonicalizeAndExtractSearchText(marshal(doc)) + if err != nil { + t.Fatalf("unexpected: %v", err) + } + for _, token := range []string{"one", "two", "H1", "H2", "a", "b"} { + if !strings.Contains(search, token) { + t.Errorf("search %q missing %q", search, token) + } + } + // No triple newlines survive normalization. + if strings.Contains(search, "\n\n\n") { + t.Errorf("search has 3+ consecutive newlines: %q", search) + } +} + +func TestSearchText_CodeBlock(t *testing.T) { + doc := map[string]any{ + "type": "doc", + "content": []any{block("codeBlock", text("go build ./..."))}, + } + _, search, _, err := CanonicalizeAndExtractSearchText(marshal(doc)) + if err != nil { + t.Fatalf("unexpected: %v", err) + } + if search != "go build ./..." { + t.Errorf("search = %q", search) + } +} + +func TestLinkDiscovery_OnlyApprovedAttrs(t *testing.T) { + doc := map[string]any{ + "type": "doc", + "content": []any{ + map[string]any{ + "type": "paragraph", + "content": []any{ + // link mark with a page-id placeholder href + map[string]any{ + "type": "text", + "text": "see other page", + "marks": []any{map[string]any{"type": "link", "attrs": map[string]any{"href": "CONF_PAGE_ID:101"}}}, + }, + // ordinary text mentioning a placeholder token (must NOT be an approved link) + map[string]any{"type": "text", "text": "the token CONF_PAGE_ID:999 appears here"}, + }, + }, + // image node with attachment placeholder src + map[string]any{"type": "image", "attrs": map[string]any{"src": "CONF_ATTACHMENT:300"}}, + }, + } + _, _, links, err := CanonicalizeAndExtractSearchText(marshal(doc)) + if err != nil { + t.Fatalf("unexpected: %v", err) + } + var approved, inText int + var sawPageID, sawAttachment bool + for _, l := range links { + if l.InText { + inText++ + continue + } + approved++ + switch l.Kind { + case LinkKindPageID: + sawPageID = true + if l.Target != "101" { + t.Errorf("page-id target = %q, want 101", l.Target) + } + case LinkKindAttachment: + sawAttachment = true + if !l.InImageSrc { + t.Errorf("attachment link should be flagged InImageSrc") + } + } + } + if !sawPageID || !sawAttachment { + t.Errorf("expected page-id and attachment approved links; got %+v", links) + } + if inText == 0 { + t.Errorf("expected an in-text placeholder to be flagged separately") + } +} + +// --- helpers --- + +func block(kind string, children ...any) map[string]any { + m := map[string]any{"type": kind} + if len(children) > 0 { + m["content"] = children + } + return m +} + +func text(s string) map[string]any { + return map[string]any{"type": "text", "text": s} +} + +func marshal(v any) string { + b, _ := json.Marshal(v) + return string(b) +} diff --git a/server/model/import.go b/server/model/import.go new file mode 100644 index 0000000..da66c44 --- /dev/null +++ b/server/model/import.go @@ -0,0 +1,448 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package model + +import ( + "net/http" + "regexp" + + mmmodel "github.com/mattermost/mattermost/server/public/model" +) + +// ImportJobState is a persisted import-job lifecycle state. The values here must exactly match the +// chk_docs_importjob_state CHECK constraint in migration 000005. +type ImportJobState string + +const ( + ImportStateAwaitingSource ImportJobState = "awaiting_source" + ImportStateWaitingSourceTurn ImportJobState = "waiting_source_turn" + ImportStateQueuedPreflight ImportJobState = "queued_preflight" + ImportStatePreflighting ImportJobState = "preflighting" + ImportStateAwaitingConfirmation ImportJobState = "awaiting_confirmation" + ImportStateQueuedImport ImportJobState = "queued_import" + ImportStateImporting ImportJobState = "importing" + ImportStateCanceling ImportJobState = "canceling" + ImportStateCompleted ImportJobState = "completed" + ImportStateCompletedWithIssues ImportJobState = "completed_with_issues" + ImportStateFailed ImportJobState = "failed" + ImportStateCanceled ImportJobState = "canceled" +) + +// validImportStates is the set matching the DB CHECK; used for model-level validation. +var validImportStates = map[ImportJobState]struct{}{ + ImportStateAwaitingSource: {}, ImportStateWaitingSourceTurn: {}, + ImportStateQueuedPreflight: {}, ImportStatePreflighting: {}, + ImportStateAwaitingConfirmation: {}, + ImportStateQueuedImport: {}, ImportStateImporting: {}, ImportStateCanceling: {}, + ImportStateCompleted: {}, ImportStateCompletedWithIssues: {}, + ImportStateFailed: {}, ImportStateCanceled: {}, +} + +// IsValid reports whether s is a known import state. +func (s ImportJobState) IsValid() bool { + _, ok := validImportStates[s] + return ok +} + +// IsTerminal reports whether s is a terminal (finished) state. Source-queue release and cleanup key +// off this predicate. +func (s ImportJobState) IsTerminal() bool { + switch s { + case ImportStateCompleted, ImportStateCompletedWithIssues, ImportStateFailed, ImportStateCanceled: + return true + default: + return false + } +} + +// OwnsSourceQueue reports whether a job in state s retains ownership of its ImportSource's +// ActiveJobId (from preflight through the end of execution). Terminal states never own the queue. +func (s ImportJobState) OwnsSourceQueue() bool { + switch s { + case ImportStateQueuedPreflight, ImportStatePreflighting, + ImportStateAwaitingConfirmation, + ImportStateQueuedImport, ImportStateImporting, ImportStateCanceling: + return true + default: + return false + } +} + +// ImportJobPhase is a free-form, human-facing progress phase label (no DB CHECK). +type ImportJobPhase string + +const ( + ImportPhaseInspecting ImportJobPhase = "inspecting" + ImportPhaseResolvingUsers ImportJobPhase = "resolving_users" + ImportPhaseComputingActions ImportJobPhase = "computing_actions" + ImportPhaseProvisioning ImportJobPhase = "provisioning_space" + ImportPhaseWritingPages ImportJobPhase = "writing_pages" + ImportPhaseFinalizing ImportJobPhase = "finalizing" +) + +// ImportTargetKind selects a new or existing Docs Space. Matches chk_docs_importjob_target. +type ImportTargetKind string + +const ( + ImportTargetNew ImportTargetKind = "new" + ImportTargetExisting ImportTargetKind = "existing" +) + +// IsValid reports whether k is a known target kind. +func (k ImportTargetKind) IsValid() bool { + return k == ImportTargetNew || k == ImportTargetExisting +} + +// ImportSourceSelectionMode selects a new or existing ImportSource. Matches +// chk_docs_importjob_source_mode (which also permits the empty string before selection). +type ImportSourceSelectionMode string + +const ( + ImportSourceModeUnset ImportSourceSelectionMode = "" + ImportSourceModeNew ImportSourceSelectionMode = "new" + ImportSourceModeExisting ImportSourceSelectionMode = "existing" +) + +// IsValid reports whether m is a known source-selection mode (including unset). +func (m ImportSourceSelectionMode) IsValid() bool { + switch m { + case ImportSourceModeUnset, ImportSourceModeNew, ImportSourceModeExisting: + return true + default: + return false + } +} + +// ImportAction is the planned/actual per-page decision recorded in staging and results. +type ImportAction string + +const ( + ImportActionCreate ImportAction = "create" + ImportActionUpdate ImportAction = "update" + ImportActionNoop ImportAction = "noop" + ImportActionPreserveLocal ImportAction = "preserve_local" + ImportActionConflict ImportAction = "conflict" + ImportActionStale ImportAction = "stale" + ImportActionBlocked ImportAction = "blocked" +) + +// Import entity, stage, and severity constants match their respective DB CHECK constraints. +const ( + ImportEntityTypePage = "page" + + ImportStageInspection = "inspection" + ImportStagePreflight = "preflight" + ImportStageExecution = "execution" + + ImportSeverityInfo = "info" + ImportSeverityWarning = "warning" + ImportSeverityError = "error" + + ImportSourceTypeConfluence = "confluence" +) + +// Field size limits enforced at the model boundary (mirroring the migration's column types). +const ( + ImportDisplayNameMaxRunes = 255 + ImportSpaceTitleMaxRunes = 128 + ImportErrorCodeMaxRunes = 64 + ImportIssueCodeMaxRunes = 64 +) + +// hexSHA256 matches exactly 64 lowercase hexadecimal characters. Every non-empty SHA-256 column is +// validated against this at the model/application boundary so a malformed or CHAR-padded value +// never enters a comparison. +var hexSHA256 = regexp.MustCompile(`^[0-9a-f]{64}$`) + +// IsValidImportHash reports whether s is empty or exactly 64 lowercase hex characters. +func IsValidImportHash(s string) bool { + return s == "" || hexSHA256.MatchString(s) +} + +// ImportSource identifies one Confluence Space chosen by a user within one target Docs Space. It — +// not organization id or space key — scopes page mappings (DOCS_ImportSource). +type ImportSource struct { + Id string `json:"id"` + SpaceId string `json:"space_id"` + SourceType string `json:"source_type"` + DisplayName string `json:"display_name"` + OrganizationId string `json:"organization_id,omitempty"` + ExternalSpaceKey string `json:"external_space_key"` + ExternalSpaceName string `json:"external_space_name"` + CreatedBy string `json:"created_by"` + CreateAt int64 `json:"create_at"` + UpdateAt int64 `json:"update_at"` + LastImportAt int64 `json:"last_import_at"` + LastSuccessfulJobId string `json:"last_successful_job_id,omitempty"` + ActiveJobId string `json:"active_job_id,omitempty"` + Props mmmodel.StringInterface `json:"props"` +} + +// ImportJob is one restartable import lifecycle (DOCS_ImportJob). Claim/lease fields are internal +// and never exposed through ImportJobView. +type ImportJob struct { + Id string `json:"id"` + ActorId string `json:"actor_id"` + TeamId string `json:"team_id"` + + TargetKind ImportTargetKind `json:"target_kind"` + TargetSpaceId string `json:"target_space_id"` + TargetSpaceExisted bool `json:"target_space_existed"` + ConfirmedSpaceTitle string `json:"confirmed_space_title,omitempty"` + ConfirmedSpaceDescription string `json:"confirmed_space_description,omitempty"` + ProvisionedChannelId string `json:"-"` + + SourceSelectionMode ImportSourceSelectionMode `json:"source_selection_mode"` + SelectedImportSourceId string `json:"selected_import_source_id,omitempty"` + SelectedSourceDisplayName string `json:"selected_source_display_name,omitempty"` + + State ImportJobState `json:"state"` + Phase ImportJobPhase `json:"phase,omitempty"` + ProgressCurrent int64 `json:"progress_current"` + ProgressTotal int64 `json:"progress_total"` + + BundleSha256 string `json:"-"` + BundleSummary mmmodel.StringInterface `json:"bundle_summary"` + PreflightSummary mmmodel.StringInterface `json:"preflight_summary"` + PreflightRevision string `json:"preflight_revision,omitempty"` + Confirmation mmmodel.StringInterface `json:"-"` + FinalSummary mmmodel.StringInterface `json:"final_summary"` + + ErrorCode string `json:"error_code,omitempty"` + ErrorMessage string `json:"-"` + CancelRequestedAt int64 `json:"cancel_requested_at"` + + ClaimToken string `json:"-"` + ClaimedBy string `json:"-"` + LeaseExpiresAt int64 `json:"-"` + HeartbeatAt int64 `json:"-"` + + CreateAt int64 `json:"create_at"` + UpdateAt int64 `json:"update_at"` + ConfirmedAt int64 `json:"confirmed_at"` + StartedAt int64 `json:"started_at"` + FinishedAt int64 `json:"finished_at"` + RetainUntil int64 `json:"-"` +} + +// ImportStagedPage is one normalized staged page (DOCS_ImportStagedPage), retained until cleanup. +type ImportStagedPage struct { + JobId string `json:"job_id"` + Ordinal int `json:"ordinal"` + ExternalId string `json:"external_id"` + ParentExternalId string `json:"parent_external_id"` + SourceOrdinal int `json:"source_ordinal"` + + Title string `json:"title"` + CanonicalBody string `json:"-"` + SearchText string `json:"-"` + SourceUserProposal string `json:"source_user_proposal"` + SourceAuthorAccountId string `json:"source_author_account_id"` + SourceCreateAt int64 `json:"source_create_at"` + SourceUpdateAt int64 `json:"source_update_at"` + SourceProps mmmodel.StringInterface `json:"source_props"` + + IncomingSourceHash string `json:"incoming_source_hash"` + PreflightCurrentHash string `json:"preflight_current_hash,omitempty"` + PreflightMappingHash string `json:"preflight_mapping_hash,omitempty"` + PreflightMappingUpdateAt int64 `json:"preflight_mapping_update_at,omitempty"` + PlannedAction ImportAction `json:"planned_action,omitempty"` + PlannedPageId string `json:"planned_page_id,omitempty"` + ResolvedUserId string `json:"resolved_user_id,omitempty"` + AuthorFallbackReason string `json:"author_fallback_reason,omitempty"` +} + +// ImportEntity is the durable page mapping (DOCS_ImportEntity), the idempotency boundary. +type ImportEntity struct { + ImportSourceId string `json:"import_source_id"` + EntityType string `json:"entity_type"` + ExternalId string `json:"external_id"` + LocalId string `json:"local_id"` + LastSourceHash string `json:"last_source_hash"` + LastAppliedHash string `json:"last_applied_hash"` + LastSourceParentExternalId string `json:"last_source_parent_external_id"` + LastSourceOrdinal int `json:"last_source_ordinal"` + FirstJobId string `json:"first_job_id"` + LastSeenJobId string `json:"last_seen_job_id"` + CreateAt int64 `json:"create_at"` + UpdateAt int64 `json:"update_at"` +} + +// ImportResultRecord is one durable entity-level outcome row (DOCS_ImportResult). +type ImportResultRecord struct { + JobId string `json:"job_id"` + Stage string `json:"stage"` + Ordinal int `json:"ordinal"` + EntityType string `json:"entity_type"` + ExternalId string `json:"external_id"` + LocalId string `json:"local_id,omitempty"` + Title string `json:"title,omitempty"` + PlannedAction ImportAction `json:"planned_action,omitempty"` + ActualAction ImportAction `json:"actual_action,omitempty"` + Outcome string `json:"outcome"` + Details mmmodel.StringInterface `json:"details,omitempty"` + CreateAt int64 `json:"create_at"` + UpdateAt int64 `json:"update_at"` +} + +// ImportIssueRecord is one durable issue row (DOCS_ImportIssue). +type ImportIssueRecord struct { + JobId string `json:"job_id"` + Stage string `json:"stage"` + Ordinal int `json:"ordinal"` + Severity string `json:"severity"` + Code string `json:"code"` + EntityType string `json:"entity_type,omitempty"` + ExternalId string `json:"external_id,omitempty"` + LocalId string `json:"local_id,omitempty"` + Title string `json:"title,omitempty"` + Message string `json:"message"` + Remediation string `json:"remediation,omitempty"` + Details mmmodel.StringInterface `json:"details,omitempty"` +} + +// --- API-safe projections --- + +// ImportProgress is the public progress projection. +type ImportProgress struct { + Phase ImportJobPhase `json:"phase,omitempty"` + Current int64 `json:"current"` + Total int64 `json:"total"` +} + +// ImportTargetView is the public target projection. +type ImportTargetView struct { + Kind ImportTargetKind `json:"kind"` + SpaceId string `json:"space_id,omitempty"` + TeamId string `json:"team_id"` + Existed bool `json:"existed"` +} + +// ImportBundleSummary is the public inspected-bundle projection. +type ImportBundleSummary struct { + Version int `json:"version"` + Source ImportReportSource `json:"source"` + SpaceDefaults ImportSpaceDefaults `json:"space_defaults"` + Counts ImportBundleCounts `json:"counts"` +} + +// ImportSpaceDefaults carries the bundle-derived, editable new-Space metadata. +type ImportSpaceDefaults struct { + Title string `json:"title"` + Description string `json:"description"` +} + +// ImportBundleCounts is the public bundle counts projection. +type ImportBundleCounts struct { + Pages int `json:"pages"` + Comments int `json:"comments"` + Attachments int `json:"attachments"` + RestrictedManifestTotal int `json:"restricted_manifest_total"` + RestrictedEmittedPages int `json:"restricted_emitted_pages"` + RestrictedManifestOnly int `json:"restricted_manifest_only"` +} + +// ImportSourceCandidate is a suggested existing ImportSource with match reasons (never auto-selected). +type ImportSourceCandidate struct { + ImportSourceId string `json:"import_source_id"` + DisplayName string `json:"display_name"` + OrganizationId string `json:"organization_id,omitempty"` + ExternalSpaceKey string `json:"external_space_key"` + MappedPageCount int `json:"mapped_page_count"` + LastImportAt int64 `json:"last_import_at"` + MatchReasons []string `json:"match_reasons"` +} + +// ImportSelectedSource is the public selected-source projection. +type ImportSelectedSource struct { + Mode ImportSourceSelectionMode `json:"mode"` + ImportSourceId string `json:"import_source_id,omitempty"` + DisplayName string `json:"display_name,omitempty"` +} + +// ImportPublicError is a scrubbed error projection (stable code only, never internal detail). +type ImportPublicError struct { + Code string `json:"code"` +} + +// ImportJobView is the API-safe projection of a job. It deliberately omits claim tokens, lease +// owners, provisioned channel IDs, internal SQL errors, bodies, and raw source props. +type ImportJobView struct { + Id string `json:"id"` + State ImportJobState `json:"state"` + Phase ImportJobPhase `json:"phase,omitempty"` + Progress ImportProgress `json:"progress"` + Target ImportTargetView `json:"target"` + Bundle ImportBundleSummary `json:"bundle"` + SourceCandidates []ImportSourceCandidate `json:"source_candidates"` + SelectedSource *ImportSelectedSource `json:"selected_source,omitempty"` + Preflight *ImportReportSummary `json:"preflight,omitempty"` + Final *ImportReportSummary `json:"final,omitempty"` + RequiredAcknowledgements []string `json:"required_acknowledgements"` + Error *ImportPublicError `json:"error,omitempty"` + CreateAt int64 `json:"create_at"` + UpdateAt int64 `json:"update_at"` + FinishedAt int64 `json:"finished_at"` +} + +// IsValid checks an ImportJob's required fields and enumerated values before insert. It does not +// validate the full state machine (transitions are enforced by compare-and-set store updates). +func (j *ImportJob) IsValid() *mmmodel.AppError { + where := "ImportJob.IsValid" + if !mmmodel.IsValidId(j.Id) { + return mmmodel.NewAppError(where, "model.import_job.is_valid.id.app_error", nil, "", http.StatusBadRequest) + } + if !mmmodel.IsValidId(j.ActorId) { + return mmmodel.NewAppError(where, "model.import_job.is_valid.actor_id.app_error", nil, "id="+j.Id, http.StatusBadRequest) + } + if !mmmodel.IsValidId(j.TeamId) { + return mmmodel.NewAppError(where, "model.import_job.is_valid.team_id.app_error", nil, "id="+j.Id, http.StatusBadRequest) + } + if !j.TargetKind.IsValid() { + return mmmodel.NewAppError(where, "model.import_job.is_valid.target_kind.app_error", nil, "id="+j.Id, http.StatusBadRequest) + } + if !mmmodel.IsValidId(j.TargetSpaceId) { + return mmmodel.NewAppError(where, "model.import_job.is_valid.target_space_id.app_error", nil, "id="+j.Id, http.StatusBadRequest) + } + if !j.SourceSelectionMode.IsValid() { + return mmmodel.NewAppError(where, "model.import_job.is_valid.source_mode.app_error", nil, "id="+j.Id, http.StatusBadRequest) + } + if !j.State.IsValid() { + return mmmodel.NewAppError(where, "model.import_job.is_valid.state.app_error", nil, "id="+j.Id, http.StatusBadRequest) + } + if j.BundleSha256 != "" && !hexSHA256.MatchString(j.BundleSha256) { + return mmmodel.NewAppError(where, "model.import_job.is_valid.bundle_sha.app_error", nil, "id="+j.Id, http.StatusBadRequest) + } + if !IsValidImportHash(j.PreflightRevision) { + return mmmodel.NewAppError(where, "model.import_job.is_valid.preflight_revision.app_error", nil, "id="+j.Id, http.StatusBadRequest) + } + if j.CreateAt == 0 || j.UpdateAt == 0 || j.RetainUntil == 0 { + return mmmodel.NewAppError(where, "model.import_job.is_valid.timestamps.app_error", nil, "id="+j.Id, http.StatusBadRequest) + } + return nil +} + +// IsValid checks an ImportSource's required fields before insert. +func (s *ImportSource) IsValid() *mmmodel.AppError { + where := "ImportSource.IsValid" + if !mmmodel.IsValidId(s.Id) { + return mmmodel.NewAppError(where, "model.import_source.is_valid.id.app_error", nil, "", http.StatusBadRequest) + } + if !mmmodel.IsValidId(s.SpaceId) { + return mmmodel.NewAppError(where, "model.import_source.is_valid.space_id.app_error", nil, "id="+s.Id, http.StatusBadRequest) + } + if s.SourceType != ImportSourceTypeConfluence { + return mmmodel.NewAppError(where, "model.import_source.is_valid.source_type.app_error", nil, "id="+s.Id, http.StatusBadRequest) + } + if !mmmodel.IsValidId(s.CreatedBy) { + return mmmodel.NewAppError(where, "model.import_source.is_valid.created_by.app_error", nil, "id="+s.Id, http.StatusBadRequest) + } + if s.ExternalSpaceKey == "" { + return mmmodel.NewAppError(where, "model.import_source.is_valid.space_key.app_error", nil, "id="+s.Id, http.StatusBadRequest) + } + if s.CreateAt == 0 || s.UpdateAt == 0 { + return mmmodel.NewAppError(where, "model.import_source.is_valid.timestamps.app_error", nil, "id="+s.Id, http.StatusBadRequest) + } + return nil +} diff --git a/server/model/import_report.go b/server/model/import_report.go new file mode 100644 index 0000000..03bd74f --- /dev/null +++ b/server/model/import_report.go @@ -0,0 +1,118 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package model + +// ImportReportVersion is the schema version of the downloadable JSON report. +const ImportReportVersion = 1 + +// ImportEntityRef identifies the entity an issue or result refers to. +type ImportEntityRef struct { + Type string `json:"type"` + ExternalId string `json:"external_id,omitempty"` + LocalId string `json:"local_id,omitempty"` + Title string `json:"title,omitempty"` +} + +// ImportIssue is one structured finding in a report. Codes are stable and documented. +type ImportIssue struct { + Stage string `json:"stage"` + Severity string `json:"severity"` + Code string `json:"code"` + Entity *ImportEntityRef `json:"entity,omitempty"` + Message string `json:"message"` + Remediation string `json:"remediation,omitempty"` + Details map[string]any `json:"details,omitempty"` +} + +// ImportResult is one entity-level outcome in a report. +type ImportResult struct { + Stage string `json:"stage"` + Entity ImportEntityRef `json:"entity"` + PlannedAction string `json:"planned_action,omitempty"` + ActualAction string `json:"actual_action,omitempty"` + Outcome string `json:"outcome"` + Details map[string]any `json:"details,omitempty"` +} + +// ImportReportSource is the report's source-identity block. +type ImportReportSource struct { + OrganizationId string `json:"organization_id"` + SpaceKey string `json:"space_key"` + SpaceName string `json:"space_name"` + ImportSourceId string `json:"import_source_id,omitempty"` +} + +// ImportReportTarget is the report's target block. +type ImportReportTarget struct { + Kind string `json:"kind"` + TeamId string `json:"team_id"` + SpaceId string `json:"space_id,omitempty"` + Existed bool `json:"existed"` +} + +// Fidelity string constants. Every report advertises exactly these values so no client can mistake +// a page-only import for a full-fidelity one. +const ( + FidelityScopePagesOnly = "pages_only" + FidelityCountedNotImported = "counted_not_imported" + FidelityRestrictedWidened = "imported_with_widened_access" + FidelityRestrictedReportedNotImported = "reported_not_imported" +) + +// ImportFidelity is the mandatory fidelity disclosure embedded in every report and job view. +type ImportFidelity struct { + Scope string `json:"scope"` + Comments string `json:"comments"` + Attachments string `json:"attachments"` + RestrictedEmittedPages string `json:"restricted_emitted_pages"` + RestrictedManifestOnlyEntries string `json:"restricted_manifest_only_entries"` + FullFidelity bool `json:"full_fidelity"` +} + +// NewImportFidelity returns the fixed fidelity disclosure for this release. +func NewImportFidelity() ImportFidelity { + return ImportFidelity{ + Scope: FidelityScopePagesOnly, + Comments: FidelityCountedNotImported, + Attachments: FidelityCountedNotImported, + RestrictedEmittedPages: FidelityRestrictedWidened, + RestrictedManifestOnlyEntries: FidelityRestrictedReportedNotImported, + FullFidelity: false, + } +} + +// ImportReportCounts aggregates action and entity counts for a report. +type ImportReportCounts struct { + Pages int `json:"pages"` + Comments int `json:"comments"` + Attachments int `json:"attachments"` + Actions map[string]int `json:"actions"` + Authors map[string]int `json:"authors,omitempty"` + Links map[string]int `json:"links,omitempty"` + IssuesBySeverity map[string]int `json:"issues_by_severity,omitempty"` +} + +// ImportReportSummary is the compact report projection embedded in ImportJobView (no per-entity +// results or issues; those stream from the report/issues endpoints). +type ImportReportSummary struct { + Stage string `json:"stage"` + GeneratedAt int64 `json:"generated_at"` + Fidelity ImportFidelity `json:"fidelity"` + Counts ImportReportCounts `json:"counts"` +} + +// ImportReport is the full downloadable report. Results and Issues stream from persisted rows so +// the endpoint need not hold them all in memory. +type ImportReport struct { + ReportVersion int `json:"report_version"` + Stage string `json:"stage"` + JobId string `json:"job_id"` + GeneratedAt int64 `json:"generated_at"` + Source ImportReportSource `json:"source"` + Target ImportReportTarget `json:"target"` + Fidelity ImportFidelity `json:"fidelity"` + Counts ImportReportCounts `json:"counts"` + Results []ImportResult `json:"results"` + Issues []ImportIssue `json:"issues"` +} diff --git a/server/model/import_test.go b/server/model/import_test.go new file mode 100644 index 0000000..b5bf543 --- /dev/null +++ b/server/model/import_test.go @@ -0,0 +1,128 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package model + +import ( + "strings" + "testing" + + mmmodel "github.com/mattermost/mattermost/server/public/model" +) + +func TestImportJobState_Predicates(t *testing.T) { + terminal := []ImportJobState{ImportStateCompleted, ImportStateCompletedWithIssues, ImportStateFailed, ImportStateCanceled} + for _, s := range terminal { + if !s.IsTerminal() { + t.Errorf("%s should be terminal", s) + } + if s.OwnsSourceQueue() { + t.Errorf("%s (terminal) must not own the source queue", s) + } + } + owning := []ImportJobState{ImportStateQueuedPreflight, ImportStatePreflighting, ImportStateAwaitingConfirmation, ImportStateQueuedImport, ImportStateImporting, ImportStateCanceling} + for _, s := range owning { + if s.IsTerminal() { + t.Errorf("%s should not be terminal", s) + } + if !s.OwnsSourceQueue() { + t.Errorf("%s should own the source queue", s) + } + } + if ImportJobState("bogus").IsValid() { + t.Errorf("bogus state should be invalid") + } + if !ImportStateAwaitingSource.IsValid() { + t.Errorf("awaiting_source should be valid") + } +} + +func TestIsValidImportHash(t *testing.T) { + valid := strings.Repeat("a", 64) + if !IsValidImportHash("") || !IsValidImportHash(valid) { + t.Errorf("empty and 64-hex should be valid") + } + for _, bad := range []string{strings.Repeat("A", 64), strings.Repeat("a", 63), strings.Repeat("a", 65), "xyz"} { + if IsValidImportHash(bad) { + t.Errorf("%q should be invalid", bad) + } + } +} + +func validJob() *ImportJob { + return &ImportJob{ + Id: mmmodel.NewId(), + ActorId: mmmodel.NewId(), + TeamId: mmmodel.NewId(), + TargetKind: ImportTargetNew, + TargetSpaceId: mmmodel.NewId(), + SourceSelectionMode: ImportSourceModeNew, + State: ImportStateQueuedPreflight, + BundleSha256: strings.Repeat("a", 64), + CreateAt: 1, + UpdateAt: 1, + RetainUntil: 2, + } +} + +func TestImportJob_IsValid(t *testing.T) { + if err := validJob().IsValid(); err != nil { + t.Fatalf("valid job rejected: %v", err) + } + + tests := map[string]func(*ImportJob){ + "bad id": func(j *ImportJob) { j.Id = "x" }, + "bad actor": func(j *ImportJob) { j.ActorId = "" }, + "bad target kind": func(j *ImportJob) { j.TargetKind = "sideways" }, + "bad target space": func(j *ImportJob) { j.TargetSpaceId = "" }, + "bad source mode": func(j *ImportJob) { j.SourceSelectionMode = "maybe" }, + "bad state": func(j *ImportJob) { j.State = "limbo" }, + "bad bundle sha": func(j *ImportJob) { j.BundleSha256 = "nothex" }, + "bad preflight rev": func(j *ImportJob) { j.PreflightRevision = "short" }, + "zero timestamps": func(j *ImportJob) { j.CreateAt = 0 }, + } + for name, mutate := range tests { + t.Run(name, func(t *testing.T) { + j := validJob() + mutate(j) + if err := j.IsValid(); err == nil { + t.Errorf("expected rejection for %q", name) + } + }) + } +} + +func TestImportSource_IsValid(t *testing.T) { + valid := &ImportSource{ + Id: mmmodel.NewId(), + SpaceId: mmmodel.NewId(), + SourceType: ImportSourceTypeConfluence, + ExternalSpaceKey: "DOCS", + CreatedBy: mmmodel.NewId(), + CreateAt: 1, + UpdateAt: 1, + } + if err := valid.IsValid(); err != nil { + t.Fatalf("valid source rejected: %v", err) + } + bad := *valid + bad.SourceType = "notion" + if err := bad.IsValid(); err == nil { + t.Errorf("non-confluence source type should be rejected") + } + bad2 := *valid + bad2.ExternalSpaceKey = "" + if err := bad2.IsValid(); err == nil { + t.Errorf("empty space key should be rejected") + } +} + +func TestNewImportFidelity(t *testing.T) { + f := NewImportFidelity() + if f.FullFidelity { + t.Errorf("full_fidelity must always be false") + } + if f.Scope != FidelityScopePagesOnly || f.Comments != FidelityCountedNotImported { + t.Errorf("unexpected fidelity: %+v", f) + } +} diff --git a/server/store/migrations/000005_create_imports.down.sql b/server/store/migrations/000005_create_imports.down.sql new file mode 100644 index 0000000..2026f7e --- /dev/null +++ b/server/store/migrations/000005_create_imports.down.sql @@ -0,0 +1,7 @@ +-- Drop in dependency order (children before parents). +DROP TABLE IF EXISTS DOCS_ImportResult; +DROP TABLE IF EXISTS DOCS_ImportIssue; +DROP TABLE IF EXISTS DOCS_ImportEntity; +DROP TABLE IF EXISTS DOCS_ImportStagedPage; +DROP TABLE IF EXISTS DOCS_ImportJob; +DROP TABLE IF EXISTS DOCS_ImportSource; diff --git a/server/store/migrations/000005_create_imports.up.sql b/server/store/migrations/000005_create_imports.up.sql new file mode 100644 index 0000000..8125103 --- /dev/null +++ b/server/store/migrations/000005_create_imports.up.sql @@ -0,0 +1,222 @@ +-- DOCS_ImportSource: one row identifies one Confluence Space as explicitly chosen by a user +-- within one target Docs Space. It — not organization id or space key — scopes page mappings. +CREATE TABLE IF NOT EXISTS DOCS_ImportSource ( + Id VARCHAR(26) PRIMARY KEY, + SpaceId VARCHAR(26) NOT NULL, + SourceType VARCHAR(32) NOT NULL DEFAULT 'confluence', + DisplayName VARCHAR(255) NOT NULL DEFAULT '', + OrganizationId TEXT NULL, + ExternalSpaceKey TEXT NOT NULL, + ExternalSpaceName TEXT NOT NULL DEFAULT '', + CreatedBy VARCHAR(26) NOT NULL, + CreateAt BIGINT NOT NULL, + UpdateAt BIGINT NOT NULL, + LastImportAt BIGINT NOT NULL DEFAULT 0, + LastSuccessfulJobId VARCHAR(26) NOT NULL DEFAULT '', + ActiveJobId VARCHAR(26) NOT NULL DEFAULT '', + Props jsonb NOT NULL DEFAULT '{}'::jsonb, + CONSTRAINT chk_docs_importsource_type CHECK (SourceType = 'confluence') +); + +CREATE INDEX IF NOT EXISTS idx_docs_importsource_space + ON DOCS_ImportSource (SpaceId, CreateAt, Id); + +-- Do not add uniqueness on organization id, space key, or display name: two Confluence +-- instances may use identical values and must remain selectable as distinct sources. +CREATE INDEX IF NOT EXISTS idx_docs_importsource_candidate + ON DOCS_ImportSource (SpaceId, SourceType, ExternalSpaceKey, OrganizationId, CreateAt); + +CREATE INDEX IF NOT EXISTS idx_docs_importsource_active_job + ON DOCS_ImportSource (ActiveJobId) + WHERE ActiveJobId <> ''; + +-- DOCS_ImportJob: one restartable import lifecycle. TargetSpaceId is generated before insert +-- (even for a new target), making target-level serialization possible without a nullable key. +CREATE TABLE IF NOT EXISTS DOCS_ImportJob ( + Id VARCHAR(26) PRIMARY KEY, + ActorId VARCHAR(26) NOT NULL, + TeamId VARCHAR(26) NOT NULL, + + TargetKind VARCHAR(16) NOT NULL, + TargetSpaceId VARCHAR(26) NOT NULL, + TargetSpaceExisted BOOLEAN NOT NULL, + ConfirmedSpaceTitle VARCHAR(128) NOT NULL DEFAULT '', + ConfirmedSpaceDescription TEXT NOT NULL DEFAULT '', + ProvisionedChannelId VARCHAR(26) NOT NULL DEFAULT '', + + SourceSelectionMode VARCHAR(16) NOT NULL DEFAULT '', + SelectedImportSourceId VARCHAR(26) NOT NULL DEFAULT '', + SelectedSourceDisplayName VARCHAR(255) NOT NULL DEFAULT '', + + State VARCHAR(32) NOT NULL, + Phase VARCHAR(64) NOT NULL DEFAULT '', + ProgressCurrent BIGINT NOT NULL DEFAULT 0, + ProgressTotal BIGINT NOT NULL DEFAULT 0, + + BundleSha256 VARCHAR(64) NOT NULL, + BundleSummary jsonb NOT NULL DEFAULT '{}'::jsonb, + PreflightSummary jsonb NOT NULL DEFAULT '{}'::jsonb, + PreflightRevision VARCHAR(64) NOT NULL DEFAULT '', + Confirmation jsonb NOT NULL DEFAULT '{}'::jsonb, + FinalSummary jsonb NOT NULL DEFAULT '{}'::jsonb, + + ErrorCode VARCHAR(64) NOT NULL DEFAULT '', + ErrorMessage TEXT NOT NULL DEFAULT '', + CancelRequestedAt BIGINT NOT NULL DEFAULT 0, + + ClaimToken VARCHAR(26) NOT NULL DEFAULT '', + ClaimedBy VARCHAR(128) NOT NULL DEFAULT '', + LeaseExpiresAt BIGINT NOT NULL DEFAULT 0, + HeartbeatAt BIGINT NOT NULL DEFAULT 0, + + CreateAt BIGINT NOT NULL, + UpdateAt BIGINT NOT NULL, + ConfirmedAt BIGINT NOT NULL DEFAULT 0, + StartedAt BIGINT NOT NULL DEFAULT 0, + FinishedAt BIGINT NOT NULL DEFAULT 0, + RetainUntil BIGINT NOT NULL, + + CONSTRAINT chk_docs_importjob_target + CHECK (TargetKind IN ('new', 'existing')), + CONSTRAINT chk_docs_importjob_source_mode + CHECK (SourceSelectionMode IN ('', 'new', 'existing')), + CONSTRAINT chk_docs_importjob_state + CHECK (State IN ( + 'awaiting_source', + 'waiting_source_turn', + 'queued_preflight', 'preflighting', + 'awaiting_confirmation', + 'queued_import', 'importing', 'canceling', + 'completed', 'completed_with_issues', + 'failed', 'canceled' + )) +); + +CREATE INDEX IF NOT EXISTS idx_docs_importjob_claim + ON DOCS_ImportJob (State, LeaseExpiresAt, CreateAt, Id) + WHERE State IN ('queued_preflight', 'preflighting', 'queued_import', 'importing', 'canceling'); + +CREATE INDEX IF NOT EXISTS idx_docs_importjob_actor + ON DOCS_ImportJob (ActorId, CreateAt DESC, Id DESC); + +CREATE INDEX IF NOT EXISTS idx_docs_importjob_target + ON DOCS_ImportJob (TargetSpaceId, CreateAt DESC, Id DESC); + +CREATE INDEX IF NOT EXISTS idx_docs_importjob_cleanup + ON DOCS_ImportJob (RetainUntil) + WHERE State IN ('completed', 'completed_with_issues', 'failed', 'canceled'); + +-- Only one queued/running execution per target Space at a time. +CREATE UNIQUE INDEX IF NOT EXISTS uq_docs_importjob_active_target + ON DOCS_ImportJob (TargetSpaceId) + WHERE State IN ('queued_import', 'importing', 'canceling'); + +-- DOCS_ImportStagedPage: temporary normalized input retained until job cleanup. +CREATE TABLE IF NOT EXISTS DOCS_ImportStagedPage ( + JobId VARCHAR(26) NOT NULL + REFERENCES DOCS_ImportJob(Id) ON DELETE CASCADE, + Ordinal INTEGER NOT NULL, + ExternalId TEXT NOT NULL, + ParentExternalId TEXT NOT NULL DEFAULT '', + SourceOrdinal INTEGER NOT NULL, + + Title TEXT NOT NULL, + CanonicalBody TEXT NOT NULL, + SearchText TEXT NOT NULL, + SourceUserProposal TEXT NOT NULL DEFAULT '', + SourceAuthorAccountId TEXT NOT NULL DEFAULT '', + SourceCreateAt BIGINT NOT NULL DEFAULT 0, + SourceUpdateAt BIGINT NOT NULL DEFAULT 0, + SourceProps jsonb NOT NULL DEFAULT '{}'::jsonb, + + IncomingSourceHash VARCHAR(64) NOT NULL, + PreflightCurrentHash VARCHAR(64) NOT NULL DEFAULT '', + PreflightMappingHash VARCHAR(64) NOT NULL DEFAULT '', + PreflightMappingUpdateAt BIGINT NOT NULL DEFAULT 0, + PlannedAction VARCHAR(32) NOT NULL DEFAULT '', + PlannedPageId VARCHAR(26) NOT NULL DEFAULT '', + ResolvedUserId VARCHAR(26) NOT NULL DEFAULT '', + AuthorFallbackReason VARCHAR(64) NOT NULL DEFAULT '', + + PRIMARY KEY (JobId, ExternalId), + UNIQUE (JobId, Ordinal) +); + +CREATE INDEX IF NOT EXISTS idx_docs_importstagedpage_order + ON DOCS_ImportStagedPage (JobId, Ordinal); + +-- DOCS_ImportEntity: the durable idempotency boundary. The same external id may exist in two +-- ImportSources even when they target the same Docs Space. +CREATE TABLE IF NOT EXISTS DOCS_ImportEntity ( + ImportSourceId VARCHAR(26) NOT NULL + REFERENCES DOCS_ImportSource(Id) ON DELETE CASCADE, + EntityType VARCHAR(32) NOT NULL, + ExternalId TEXT NOT NULL, + LocalId VARCHAR(26) NOT NULL, + + LastSourceHash VARCHAR(64) NOT NULL, + LastAppliedHash VARCHAR(64) NOT NULL, + LastSourceParentExternalId TEXT NOT NULL DEFAULT '', + LastSourceOrdinal INTEGER NOT NULL DEFAULT 0, + FirstJobId VARCHAR(26) NOT NULL, + LastSeenJobId VARCHAR(26) NOT NULL, + CreateAt BIGINT NOT NULL, + UpdateAt BIGINT NOT NULL, + + PRIMARY KEY (ImportSourceId, EntityType, ExternalId), + CONSTRAINT chk_docs_importentity_type CHECK (EntityType = 'page') +); + +-- One local Page can map to at most one ImportSource. +CREATE UNIQUE INDEX IF NOT EXISTS uq_docs_importentity_local_page + ON DOCS_ImportEntity (LocalId) + WHERE EntityType = 'page'; + +-- DOCS_ImportIssue: individual structured issue rows (never thousands packed into one JSONB field). +CREATE TABLE IF NOT EXISTS DOCS_ImportIssue ( + JobId VARCHAR(26) NOT NULL + REFERENCES DOCS_ImportJob(Id) ON DELETE CASCADE, + Stage VARCHAR(16) NOT NULL, + Ordinal INTEGER NOT NULL, + Severity VARCHAR(16) NOT NULL, + Code VARCHAR(64) NOT NULL, + EntityType VARCHAR(32) NOT NULL DEFAULT '', + ExternalId TEXT NOT NULL DEFAULT '', + LocalId VARCHAR(26) NOT NULL DEFAULT '', + Title TEXT NOT NULL DEFAULT '', + Message TEXT NOT NULL, + Remediation TEXT NOT NULL DEFAULT '', + Details jsonb NOT NULL DEFAULT '{}'::jsonb, + + PRIMARY KEY (JobId, Stage, Ordinal), + CONSTRAINT chk_docs_importissue_stage CHECK (Stage IN ('inspection', 'preflight', 'execution')), + CONSTRAINT chk_docs_importissue_severity CHECK (Severity IN ('info', 'warning', 'error')) +); + +CREATE INDEX IF NOT EXISTS idx_docs_importissue_page + ON DOCS_ImportIssue (JobId, Stage, Severity, Ordinal); + +-- DOCS_ImportResult: durable entity-level outcomes, kept separate from staging so the report can +-- enumerate creates/updates/no-ops after staged bodies are purged. Never store bodies here. +CREATE TABLE IF NOT EXISTS DOCS_ImportResult ( + JobId VARCHAR(26) NOT NULL + REFERENCES DOCS_ImportJob(Id) ON DELETE CASCADE, + Stage VARCHAR(16) NOT NULL, + Ordinal INTEGER NOT NULL, + EntityType VARCHAR(32) NOT NULL DEFAULT 'page', + ExternalId TEXT NOT NULL, + LocalId VARCHAR(26) NOT NULL DEFAULT '', + Title TEXT NOT NULL DEFAULT '', + PlannedAction VARCHAR(32) NOT NULL DEFAULT '', + ActualAction VARCHAR(32) NOT NULL DEFAULT '', + Outcome VARCHAR(32) NOT NULL DEFAULT '', + Details jsonb NOT NULL DEFAULT '{}'::jsonb, + CreateAt BIGINT NOT NULL, + UpdateAt BIGINT NOT NULL, + + PRIMARY KEY (JobId, Stage, Ordinal), + CONSTRAINT chk_docs_importresult_stage CHECK (Stage IN ('preflight', 'execution')) +); + +CREATE INDEX IF NOT EXISTS idx_docs_importresult_page + ON DOCS_ImportResult (JobId, Stage, Ordinal); From 6a8e50dcb73c1895af2b96aaff14b2242fa82668 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20Vay=C3=A1?= Date: Fri, 24 Jul 2026 12:54:05 +0200 Subject: [PATCH 2/5] Confluence import: fix six review findings in importer + models 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) --- server/importer/inspect.go | 54 ++++++++++++++++++++++---- server/importer/inspect_test.go | 55 +++++++++++++++++++++++--- server/importer/links.go | 69 ++++++++++++++++++--------------- server/importer/tiptap.go | 29 +++++++++----- server/importer/tiptap_test.go | 25 ++++++++++-- server/model/import.go | 38 ++++++++++++++++++ server/model/import_test.go | 31 +++++++++++++++ 7 files changed, 241 insertions(+), 60 deletions(-) diff --git a/server/importer/inspect.go b/server/importer/inspect.go index 1b8cbfa..93e20dd 100644 --- a/server/importer/inspect.go +++ b/server/importer/inspect.go @@ -7,8 +7,18 @@ import ( "encoding/json" "fmt" "strings" + "unicode/utf8" + + "github.com/mattermost/mattermost-plugin-docs/server/model" ) +// MaxManifestWarnings bounds how many producer manifest warnings are materialized as inspection +// issues. A valid sub-limit manifest could otherwise carry hundreds of thousands of warnings; each +// copied into an issue struct, that would let a single upload retain a large multiple of the +// manifest size, and concurrent uploads could exhaust plugin memory. Warnings beyond the cap are +// summarized in one aggregate issue rather than materialized individually. +const MaxManifestWarnings = 1000 + // Manifest mirrors the fields of the producer's import-manifest.json that the importer reads. // Unknown fields are ignored (forward-compatible). type Manifest struct { @@ -95,6 +105,7 @@ const ( InspectErrPageMissingID = "page_missing_external_id" InspectErrDuplicatePageID = "page_duplicate_external_id" InspectErrPageMissingTitle = "page_missing_title" + InspectErrPageTitleTooLong = "page_title_too_long" InspectErrParentNotSeen = "page_parent_not_seen" InspectErrCycle = "page_cycle" InspectErrDepthExceeded = "page_depth_exceeded" @@ -234,13 +245,28 @@ func Inspect(contents *ArchiveContents, opts InspectOptions) (*InspectionResult, SpaceName: manifest.Source.SpaceName, } - // Copy manifest warnings into issues. - for _, w := range manifest.Warnings { + // Copy manifest warnings into issues, bounded by MaxManifestWarnings so a manifest packed with + // warnings cannot force unbounded issue allocation. + warnings := manifest.Warnings + suppressed := 0 + if len(warnings) > MaxManifestWarnings { + suppressed = len(warnings) - MaxManifestWarnings + warnings = warnings[:MaxManifestWarnings] + } + for _, w := range warnings { res.Issues = append(res.Issues, InspectionIssue{ Severity: SeverityWarning, Code: IssueManifestWarning, Message: w, Remediation: "Review the producer warning; it does not block import.", }) } + if suppressed > 0 { + res.Issues = append(res.Issues, InspectionIssue{ + Severity: SeverityWarning, Code: IssueManifestWarning, + Message: fmt.Sprintf("%d additional manifest warnings were suppressed (limit %d)", suppressed, MaxManifestWarnings), + Remediation: "Only the first manifest warnings are reported individually.", + Details: map[string]any{"suppressed": suppressed, "limit": MaxManifestWarnings}, + }) + } // Attachments are never verified in this release. if manifest.Checksums.AttachmentsSha256 != "" { res.Issues = append(res.Issues, InspectionIssue{ @@ -439,6 +465,11 @@ func normalizePage( if title == "" { return nil, inspectErr(InspectErrPageMissingTitle, "line %d: page %q is missing a title", lineNo, externalID) } + // Reject an over-long title at inspection so it fails early here rather than at execution, where + // Page.IsValid enforces the same PageTitleMaxRunes bound. + if utf8.RuneCountInString(title) > model.PageTitleMaxRunes { + return nil, inspectErr(InspectErrPageTitleTooLong, "line %d: page %q title exceeds %d runes", lineNo, externalID, model.PageTitleMaxRunes) + } // Space key must match the manifest. if err := requireSameSpaceKey(fmt.Sprintf("page %q space_import_source_id", externalID), stringOrEmpty(page.SpaceImportSourceID), manifest.Source.SpaceKey); err != nil { @@ -559,12 +590,19 @@ func allowlistSourceProps(props map[string]any) map[string]any { return out } +// MaxHierarchyDepth is the maximum page depth the importer accepts, mirroring app.MaxPageDepth: a +// root page is depth 1, so a chain of 10 pages is the deepest allowed and an 11-page chain is +// rejected. The value is duplicated (rather than imported from the app package) to keep this pure +// package free of a dependency cycle; both must move together. +const MaxHierarchyDepth = 10 + // verifyHierarchy independently recomputes each page's depth from the parent map, rejecting cycles, -// missing parents, and depth greater than 10. It does not trust any producer flattening claim. +// missing parents, and depth greater than MaxHierarchyDepth. It counts the root as depth 1 so the +// bound matches the app-layer depth enforced at execution, and does not trust any producer +// flattening claim. func verifyHierarchy(pages []StagedPage, parentOf map[string]string) error { - const maxDepth = 10 for _, p := range pages { - depth := 0 + depth := 1 // the page itself counts as one level; root is depth 1 cur := p.ExternalID for { parent := parentOf[cur] @@ -575,10 +613,10 @@ func verifyHierarchy(pages []StagedPage, parentOf map[string]string) error { return inspectErr(InspectErrParentNotSeen, "page %q references missing parent %q", p.ExternalID, parent) } depth++ - if depth > maxDepth { - return inspectErr(InspectErrDepthExceeded, "page %q exceeds maximum hierarchy depth of %d", p.ExternalID, maxDepth) + if depth > MaxHierarchyDepth { + return inspectErr(InspectErrDepthExceeded, "page %q exceeds maximum hierarchy depth of %d", p.ExternalID, MaxHierarchyDepth) } - if depth > len(parentOf) { + if depth > len(parentOf)+1 { return inspectErr(InspectErrCycle, "page %q is part of a parent cycle", p.ExternalID) } cur = parent diff --git a/server/importer/inspect_test.go b/server/importer/inspect_test.go index fed36a2..d9bc773 100644 --- a/server/importer/inspect_test.go +++ b/server/importer/inspect_test.go @@ -202,34 +202,35 @@ func TestInspect_MissingParent(t *testing.T) { } func TestInspect_DepthExceeded(t *testing.T) { - // Build a chain of 12 pages (depth 11 at the deepest) which must be rejected. + // A chain of 11 pages puts the deepest page at depth 11 (root is depth 1), which exceeds the + // depth-10 limit and must be rejected — matching the app-layer bound enforced at execution. lines := []string{versionLine(), spaceLine()} prev := "" - for i := 0; i <= 11; i++ { + for i := 0; i <= 10; i++ { id := string(rune('a'+i)) + "id" lines = append(lines, pageLine(t, id, prev, "P", docString("x"))) prev = id } lines = append(lines, resolveLine()) jsonl := joinLines(lines...) - _, err := newBundle(jsonl, baseManifest(12, 0, 0)).inspect(t, InspectOptions{}) + _, err := newBundle(jsonl, baseManifest(11, 0, 0)).inspect(t, InspectOptions{}) if got := inspectErrCode(err); got != InspectErrDepthExceeded { t.Fatalf("code = %q, want %q", got, InspectErrDepthExceeded) } } func TestInspect_DepthTenAllowed(t *testing.T) { - // A chain of 11 pages has a maximum depth of 10, which is allowed. + // A chain of 10 pages reaches depth 10 (root is depth 1), the deepest allowed. lines := []string{versionLine(), spaceLine()} prev := "" - for i := 0; i <= 10; i++ { + for i := 0; i <= 9; i++ { id := string(rune('a'+i)) + "id" lines = append(lines, pageLine(t, id, prev, "P", docString("x"))) prev = id } lines = append(lines, resolveLine()) jsonl := joinLines(lines...) - _, err := newBundle(jsonl, baseManifest(11, 0, 0)).inspect(t, InspectOptions{}) + _, err := newBundle(jsonl, baseManifest(10, 0, 0)).inspect(t, InspectOptions{}) if err != nil { t.Fatalf("depth 10 should be allowed, got %v", err) } @@ -317,6 +318,48 @@ func TestInspect_SpaceKeyMismatch(t *testing.T) { } } +func TestInspect_TitleTooLong(t *testing.T) { + longTitle := strings.Repeat("x", 300) // > model.PageTitleMaxRunes (255) + jsonl := joinLines(versionLine(), spaceLine(), + pageLine(t, "100", "", longTitle, docString("x")), resolveLine()) + _, err := newBundle(jsonl, baseManifest(1, 0, 0)).inspect(t, InspectOptions{}) + if got := inspectErrCode(err); got != InspectErrPageTitleTooLong { + t.Fatalf("code = %q, want %q", got, InspectErrPageTitleTooLong) + } +} + +func TestInspect_ManifestWarningsCapped(t *testing.T) { + b := validBundle(t) + total := MaxManifestWarnings + 500 + b.manifest.Warnings = make([]string, total) + for i := range b.manifest.Warnings { + b.manifest.Warnings[i] = "w" + } + res, err := b.inspect(t, InspectOptions{}) + if err != nil { + t.Fatalf("unexpected: %v", err) + } + warnIssues := 0 + sawSuppressionNote := false + for _, is := range res.Issues { + if is.Code == IssueManifestWarning { + warnIssues++ + if is.Details != nil { + if _, ok := is.Details["suppressed"]; ok { + sawSuppressionNote = true + } + } + } + } + // At most MaxManifestWarnings individual warnings plus one aggregate suppression note. + if warnIssues > MaxManifestWarnings+1 { + t.Errorf("manifest warning issues = %d, want <= %d", warnIssues, MaxManifestWarnings+1) + } + if !sawSuppressionNote { + t.Errorf("expected an aggregate suppression issue when warnings exceed the cap") + } +} + func TestInspect_InvalidTipTap(t *testing.T) { jsonl := joinLines(versionLine(), spaceLine(), pageLine(t, "100", "", "H", `{"type":"notdoc"}`), resolveLine()) diff --git a/server/importer/links.go b/server/importer/links.go index dab1c2d..6ea91ec 100644 --- a/server/importer/links.go +++ b/server/importer/links.go @@ -3,11 +3,12 @@ package importer -import "strings" +import "regexp" -// Confluence link placeholder prefixes the producer emits inside TipTap link mark hrefs and image -// src attributes. V1 discovers these structurally but never rewrites them: there is no canonical -// Docs reader URL in this repository yet. +// Confluence link placeholder names the mmetl producer emits inside TipTap link mark hrefs and +// image src attributes. The producer wraps them in double braces, e.g. "{{CONF_PAGE_ID:101}}" +// (see mmetl services/confluence/links.go). V1 discovers these structurally but never rewrites +// them: there is no canonical Docs reader URL in this repository yet. const ( PlaceholderPageID = "CONF_PAGE_ID" PlaceholderPageTitle = "CONF_PAGE_TITLE" @@ -26,8 +27,8 @@ const ( ) // DiscoveredLink is one placeholder found in an approved attribute (link mark href or image src). -// Raw is the exact attribute value; Target is the portion after the placeholder prefix and colon -// (e.g. the page ID or title), when present. +// Raw is the exact attribute value; Target is the placeholder's argument (e.g. the page ID or +// title) with the braces and prefix removed. type DiscoveredLink struct { Kind LinkKind `json:"kind"` Raw string `json:"raw"` @@ -40,23 +41,7 @@ type DiscoveredLink struct { InText bool `json:"in_text"` } -// classifyPlaceholder inspects an attribute value and returns a DiscoveredLink when it begins with -// a recognized placeholder prefix, plus ok=true. A value that merely contains a placeholder token -// somewhere other than the start is not treated as a placeholder here (ordinary text is reported -// separately by the caller). -func classifyPlaceholder(value string, inImageSrc bool) (DiscoveredLink, bool) { - for prefix, kind := range placeholderKinds { - if value == prefix || strings.HasPrefix(value, prefix+":") { - target := "" - if len(value) > len(prefix)+1 { - target = value[len(prefix)+1:] - } - return DiscoveredLink{Kind: kind, Raw: value, Target: target, InImageSrc: inImageSrc}, true - } - } - return DiscoveredLink{}, false -} - +// placeholderKinds maps a recognized link/attachment placeholder name to its kind. var placeholderKinds = map[string]LinkKind{ PlaceholderPageID: LinkKindPageID, PlaceholderPageTitle: LinkKindPageTitle, @@ -64,14 +49,34 @@ var placeholderKinds = map[string]LinkKind{ PlaceholderAttachment: LinkKindAttachment, } -// containsPlaceholderToken reports whether s contains any placeholder prefix token anywhere. Used -// to flag placeholders that appear in ordinary text (which V1 never rewrites) so the report can -// note them. -func containsPlaceholderToken(s string) bool { - for prefix := range placeholderKinds { - if strings.Contains(s, prefix) { - return true - } +// linkPlaceholderRe matches a producer link/attachment placeholder of the form +// "{{CONF_PAGE_ID:target}}", capturing the placeholder name and its target argument. The producer +// URL-escapes the target, so it never itself contains "}". +var linkPlaceholderRe = regexp.MustCompile(`\{\{(CONF_PAGE_ID|CONF_PAGE_TITLE|CONF_FILE|CONF_ATTACHMENT):([^}]*)\}\}`) + +// anyPlaceholderRe matches any Confluence placeholder token (including e.g. CONF_USER) so a +// placeholder left in ordinary text can be flagged even when it is not one of the link kinds. +var anyPlaceholderRe = regexp.MustCompile(`\{\{CONF_[A-Z_]+:[^}]*\}\}`) + +// classifyPlaceholder inspects an approved attribute value (a link href or image src) and returns a +// DiscoveredLink when it contains a recognized "{{CONF_...:target}}" placeholder, plus ok=true. The +// producer sets the whole attribute to the placeholder, but this tolerates surrounding text by +// matching the first placeholder anywhere in the value. +func classifyPlaceholder(value string, inImageSrc bool) (DiscoveredLink, bool) { + m := linkPlaceholderRe.FindStringSubmatch(value) + if m == nil { + return DiscoveredLink{}, false } - return false + return DiscoveredLink{ + Kind: placeholderKinds[m[1]], + Raw: value, + Target: m[2], + InImageSrc: inImageSrc, + }, true +} + +// containsPlaceholderToken reports whether s contains any Confluence "{{CONF_...:...}}" placeholder +// token. Used to flag placeholders that appear in ordinary text (which V1 never rewrites). +func containsPlaceholderToken(s string) bool { + return anyPlaceholderRe.MatchString(s) } diff --git a/server/importer/tiptap.go b/server/importer/tiptap.go index 508683f..c4b0440 100644 --- a/server/importer/tiptap.go +++ b/server/importer/tiptap.go @@ -29,6 +29,7 @@ func tiptapErr(code, format string, args ...any) *TipTapError { const ( TipTapErrInvalidJSON = "tiptap_invalid_json" TipTapErrNotDoc = "tiptap_not_doc" + TipTapErrMissingType = "tiptap_missing_type" TipTapErrBadContent = "tiptap_bad_content" TipTapErrBadMarks = "tiptap_bad_marks" TipTapErrBadText = "tiptap_bad_text" @@ -126,7 +127,14 @@ func (w *tiptapWalker) walkNode(node map[string]any, depth int) error { return tiptapErr(TipTapErrTooDeep, "document nesting exceeds depth %d", MaxTipTapDepth) } - nodeType, _ := node["type"].(string) + // Every node must carry a non-empty string type. A missing or empty type is not an "unknown + // type" (which is preserved) but a structurally invalid node ProseMirror/TipTap clients cannot + // deserialize. + rawType, hasType := node["type"] + nodeType, typeIsString := rawType.(string) + if !hasType || !typeIsString || nodeType == "" { + return tiptapErr(TipTapErrMissingType, "node is missing a non-empty string type") + } // Marks (if present) must be an array; scan link marks for placeholder hrefs. if rawMarks, present := node["marks"]; present { @@ -155,15 +163,16 @@ func (w *tiptapWalker) walkNode(node map[string]any, depth int) error { switch nodeType { case nodeTypeText: rawText, present := node["text"] - if present { - text, ok := rawText.(string) - if !ok { - return tiptapErr(TipTapErrBadText, "text node has a non-string text field") - } - w.search.WriteString(text) - if containsPlaceholderToken(text) { - w.links = append(w.links, DiscoveredLink{Raw: text, InText: true}) - } + if !present { + return tiptapErr(TipTapErrBadText, "text node is missing its text field") + } + text, ok := rawText.(string) + if !ok { + return tiptapErr(TipTapErrBadText, "text node has a non-string text field") + } + w.search.WriteString(text) + if containsPlaceholderToken(text) { + w.links = append(w.links, DiscoveredLink{Raw: text, InText: true}) } case nodeTypeHardBreak: w.search.WriteByte('\n') diff --git a/server/importer/tiptap_test.go b/server/importer/tiptap_test.go index 22dc60c..211f34c 100644 --- a/server/importer/tiptap_test.go +++ b/server/importer/tiptap_test.go @@ -40,6 +40,23 @@ func TestCanonicalize_RejectsNonArrayContent(t *testing.T) { } } +func TestCanonicalize_RejectsMissingNodeType(t *testing.T) { + // A child node with no type is structurally invalid, not an unknown type. + body := `{"type":"doc","content":[{"attrs":{"x":1},"content":[{"type":"text","text":"hi"}]}]}` + _, _, _, err := CanonicalizeAndExtractSearchText(body) + if te, ok := err.(*TipTapError); !ok || te.Code != TipTapErrMissingType { + t.Fatalf("err = %v, want %s", err, TipTapErrMissingType) + } +} + +func TestCanonicalize_RejectsTextNodeWithoutText(t *testing.T) { + body := `{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text"}]}]}` + _, _, _, err := CanonicalizeAndExtractSearchText(body) + if te, ok := err.(*TipTapError); !ok || te.Code != TipTapErrBadText { + t.Fatalf("err = %v, want %s", err, TipTapErrBadText) + } +} + func TestSearchText_ParagraphsHeadingsHardBreak(t *testing.T) { doc := map[string]any{ "type": "doc", @@ -115,18 +132,18 @@ func TestLinkDiscovery_OnlyApprovedAttrs(t *testing.T) { map[string]any{ "type": "paragraph", "content": []any{ - // link mark with a page-id placeholder href + // link mark with a page-id placeholder href (producer emits braced form) map[string]any{ "type": "text", "text": "see other page", - "marks": []any{map[string]any{"type": "link", "attrs": map[string]any{"href": "CONF_PAGE_ID:101"}}}, + "marks": []any{map[string]any{"type": "link", "attrs": map[string]any{"href": "{{CONF_PAGE_ID:101}}"}}}, }, // ordinary text mentioning a placeholder token (must NOT be an approved link) - map[string]any{"type": "text", "text": "the token CONF_PAGE_ID:999 appears here"}, + map[string]any{"type": "text", "text": "the token {{CONF_PAGE_ID:999}} appears here"}, }, }, // image node with attachment placeholder src - map[string]any{"type": "image", "attrs": map[string]any{"src": "CONF_ATTACHMENT:300"}}, + map[string]any{"type": "image", "attrs": map[string]any{"src": "{{CONF_ATTACHMENT:300}}"}}, }, } _, _, links, err := CanonicalizeAndExtractSearchText(marshal(doc)) diff --git a/server/model/import.go b/server/model/import.go index da66c44..4a4bb30 100644 --- a/server/model/import.go +++ b/server/model/import.go @@ -6,6 +6,7 @@ package model import ( "net/http" "regexp" + "unicode/utf8" mmmodel "github.com/mattermost/mattermost/server/public/model" ) @@ -417,6 +418,17 @@ func (j *ImportJob) IsValid() *mmmodel.AppError { if !IsValidImportHash(j.PreflightRevision) { return mmmodel.NewAppError(where, "model.import_job.is_valid.preflight_revision.app_error", nil, "id="+j.Id, http.StatusBadRequest) } + // Enforce the string-length bounds that back the DB VARCHAR columns, so an over-long value is + // rejected here rather than as an opaque PostgreSQL constraint violation on insert. + if utf8.RuneCountInString(j.ConfirmedSpaceTitle) > ImportSpaceTitleMaxRunes { + return mmmodel.NewAppError(where, "model.import_job.is_valid.space_title_length.app_error", map[string]any{"MaxLength": ImportSpaceTitleMaxRunes}, "id="+j.Id, http.StatusBadRequest) + } + if utf8.RuneCountInString(j.SelectedSourceDisplayName) > ImportDisplayNameMaxRunes { + return mmmodel.NewAppError(where, "model.import_job.is_valid.source_display_name_length.app_error", map[string]any{"MaxLength": ImportDisplayNameMaxRunes}, "id="+j.Id, http.StatusBadRequest) + } + if utf8.RuneCountInString(j.ErrorCode) > ImportErrorCodeMaxRunes { + return mmmodel.NewAppError(where, "model.import_job.is_valid.error_code_length.app_error", map[string]any{"MaxLength": ImportErrorCodeMaxRunes}, "id="+j.Id, http.StatusBadRequest) + } if j.CreateAt == 0 || j.UpdateAt == 0 || j.RetainUntil == 0 { return mmmodel.NewAppError(where, "model.import_job.is_valid.timestamps.app_error", nil, "id="+j.Id, http.StatusBadRequest) } @@ -441,8 +453,34 @@ func (s *ImportSource) IsValid() *mmmodel.AppError { if s.ExternalSpaceKey == "" { return mmmodel.NewAppError(where, "model.import_source.is_valid.space_key.app_error", nil, "id="+s.Id, http.StatusBadRequest) } + if utf8.RuneCountInString(s.DisplayName) > ImportDisplayNameMaxRunes { + return mmmodel.NewAppError(where, "model.import_source.is_valid.display_name_length.app_error", map[string]any{"MaxLength": ImportDisplayNameMaxRunes}, "id="+s.Id, http.StatusBadRequest) + } if s.CreateAt == 0 || s.UpdateAt == 0 { return mmmodel.NewAppError(where, "model.import_source.is_valid.timestamps.app_error", nil, "id="+s.Id, http.StatusBadRequest) } return nil } + +// IsValid checks an ImportIssueRecord's enumerated values and the code-length bound that backs the +// DOCS_ImportIssue.Code VARCHAR(64) column, so an over-long or unknown code is rejected before insert. +func (r *ImportIssueRecord) IsValid() *mmmodel.AppError { + where := "ImportIssueRecord.IsValid" + switch r.Stage { + case ImportStageInspection, ImportStagePreflight, ImportStageExecution: + default: + return mmmodel.NewAppError(where, "model.import_issue.is_valid.stage.app_error", nil, "", http.StatusBadRequest) + } + switch r.Severity { + case ImportSeverityInfo, ImportSeverityWarning, ImportSeverityError: + default: + return mmmodel.NewAppError(where, "model.import_issue.is_valid.severity.app_error", nil, "", http.StatusBadRequest) + } + if r.Code == "" || utf8.RuneCountInString(r.Code) > ImportIssueCodeMaxRunes { + return mmmodel.NewAppError(where, "model.import_issue.is_valid.code.app_error", map[string]any{"MaxLength": ImportIssueCodeMaxRunes}, "", http.StatusBadRequest) + } + if r.Message == "" { + return mmmodel.NewAppError(where, "model.import_issue.is_valid.message.app_error", nil, "", http.StatusBadRequest) + } + return nil +} diff --git a/server/model/import_test.go b/server/model/import_test.go index b5bf543..46282ae 100644 --- a/server/model/import_test.go +++ b/server/model/import_test.go @@ -80,6 +80,9 @@ func TestImportJob_IsValid(t *testing.T) { "bad bundle sha": func(j *ImportJob) { j.BundleSha256 = "nothex" }, "bad preflight rev": func(j *ImportJob) { j.PreflightRevision = "short" }, "zero timestamps": func(j *ImportJob) { j.CreateAt = 0 }, + "long space title": func(j *ImportJob) { j.ConfirmedSpaceTitle = strings.Repeat("x", ImportSpaceTitleMaxRunes+1) }, + "long source name": func(j *ImportJob) { j.SelectedSourceDisplayName = strings.Repeat("x", ImportDisplayNameMaxRunes+1) }, + "long error code": func(j *ImportJob) { j.ErrorCode = strings.Repeat("x", ImportErrorCodeMaxRunes+1) }, } for name, mutate := range tests { t.Run(name, func(t *testing.T) { @@ -115,6 +118,34 @@ func TestImportSource_IsValid(t *testing.T) { if err := bad2.IsValid(); err == nil { t.Errorf("empty space key should be rejected") } + bad3 := *valid + bad3.DisplayName = strings.Repeat("x", ImportDisplayNameMaxRunes+1) + if err := bad3.IsValid(); err == nil { + t.Errorf("over-long display name should be rejected") + } +} + +func TestImportIssueRecord_IsValid(t *testing.T) { + valid := &ImportIssueRecord{Stage: ImportStagePreflight, Severity: ImportSeverityWarning, Code: "some_code", Message: "m"} + if err := valid.IsValid(); err != nil { + t.Fatalf("valid issue rejected: %v", err) + } + tests := map[string]func(*ImportIssueRecord){ + "bad stage": func(r *ImportIssueRecord) { r.Stage = "nowhere" }, + "bad severity": func(r *ImportIssueRecord) { r.Severity = "meh" }, + "empty code": func(r *ImportIssueRecord) { r.Code = "" }, + "long code": func(r *ImportIssueRecord) { r.Code = strings.Repeat("c", ImportIssueCodeMaxRunes+1) }, + "empty msg": func(r *ImportIssueRecord) { r.Message = "" }, + } + for name, mutate := range tests { + t.Run(name, func(t *testing.T) { + r := *valid + mutate(&r) + if err := r.IsValid(); err == nil { + t.Errorf("expected rejection for %q", name) + } + }) + } } func TestNewImportFidelity(t *testing.T) { From f435790c87c08325268f2aa3e9ad09d741599b0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20Vay=C3=A1?= Date: Mon, 27 Jul 2026 12:38:01 +0200 Subject: [PATCH 3/5] Confluence import: track-A review fixes (migration renumber + hardening) 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) --- server/importer/archive.go | 38 ++++-- server/importer/inspect.go | 92 ++++++++++--- server/importer/inspect_test.go | 125 +++++++++++++++++- server/importer/testhelpers_test.go | 31 +++++ server/model/import.go | 6 +- server/model/import_test.go | 1 + ...own.sql => 000006_create_imports.down.sql} | 0 ...ts.up.sql => 000006_create_imports.up.sql} | 0 8 files changed, 264 insertions(+), 29 deletions(-) rename server/store/migrations/{000005_create_imports.down.sql => 000006_create_imports.down.sql} (100%) rename server/store/migrations/{000005_create_imports.up.sql => 000006_create_imports.up.sql} (100%) diff --git a/server/importer/archive.go b/server/importer/archive.go index bd945d3..2b095ad 100644 --- a/server/importer/archive.go +++ b/server/importer/archive.go @@ -10,6 +10,7 @@ import ( "fmt" "io" "io/fs" + "path" "strings" ) @@ -124,33 +125,39 @@ func InspectArchive(r io.ReaderAt, n int64) (*ArchiveContents, error) { } rawSeen[raw] = struct{}{} - name := raw // only "/" separators are permitted, verified above - - if unsafeErr := checkUnsafeName(name); unsafeErr != nil { + // Reject traversal/absolute/drive names on the raw form first, so path.Clean below cannot + // mask a ".." segment by collapsing it before it is detected. + if unsafeErr := checkUnsafeName(raw); unsafeErr != nil { return nil, unsafeErr } + + // Genuinely normalize before duplicate detection: collapse "." and repeated "/" segments so + // e.g. "data//x" and "data/x" are recognized as the same entry. Only "/" separators are + // present (backslashes are rejected above). The trailing slash is preserved so a directory + // entry stays distinct from a same-named file. + name := normalizeEntryName(raw) if _, dup := normSeen[name]; dup { return nil, archiveErr(ArchiveErrDuplicateEntry, "archive contains duplicate normalized entry %q", name) } normSeen[name] = struct{}{} + // Validate mode, encryption, and compression method for every file entry — including the + // data/ payloads we never open — so an unsafe entry is rejected uniformly rather than only + // for the two entries whose bytes are read. isDir := strings.HasSuffix(name, "/") if !isDir { if modeErr := checkEntryMode(f); modeErr != nil { return nil, modeErr } + if methodErr := checkSupportedMethod(f); methodErr != nil { + return nil, methodErr + } } switch { case name == entryJSONL: - if methodErr := checkSupportedMethod(f); methodErr != nil { - return nil, methodErr - } jsonlFile = f case name == entryManifest: - if methodErr := checkSupportedMethod(f); methodErr != nil { - return nil, methodErr - } manifestFile = f case name == entryDataDir || strings.HasPrefix(name, entryDataDir): // data/ files are permitted but never opened. @@ -193,6 +200,19 @@ func InspectArchive(r io.ReaderAt, n int64) (*ArchiveContents, error) { }, nil } +// normalizeEntryName canonicalizes a "/"-separated ZIP entry name for duplicate detection by +// collapsing "." and repeated-slash segments via path.Clean, preserving a trailing slash so a +// directory entry does not alias a same-named file. It must be called only after checkUnsafeName +// has rejected ".." segments, since path.Clean would otherwise resolve them away. +func normalizeEntryName(name string) string { + isDir := strings.HasSuffix(name, "/") + cleaned := path.Clean(name) + if isDir && cleaned != "/" && cleaned != "." { + cleaned += "/" + } + return cleaned +} + // checkUnsafeName rejects path traversal and platform-specific unsafe names on a "/"-separated // entry name. func checkUnsafeName(name string) error { diff --git a/server/importer/inspect.go b/server/importer/inspect.go index 93e20dd..0551a99 100644 --- a/server/importer/inspect.go +++ b/server/importer/inspect.go @@ -111,6 +111,7 @@ const ( InspectErrDepthExceeded = "page_depth_exceeded" InspectErrTipTap = "page_content_invalid" InspectErrSpaceKeyMismatch = "space_key_mismatch" + InspectErrSpaceKeyMissing = "space_key_missing" InspectErrCommentMissingPageID = "comment_missing_page_id" InspectErrAttachmentPath = "attachment_invalid_path" InspectErrHash = "hash_failed" @@ -132,7 +133,11 @@ const ( IssueSourceUpdateAtInvalid = "source_update_at_invalid" IssueManifestCountMismatch = "manifest_count_mismatch" IssuePlaceholderInText = "placeholder_in_text_not_rewritten" - IssueAttachmentNotImported = "attachment_placeholder_not_imported" + // IssueAttachmentsNotImported flags a page that carries attachment records, none of which are + // imported in this release. This is the plan's partial-scope code (section 20.2), distinct from + // the attachment_placeholder_not_imported link code used when a CONF_ATTACHMENT placeholder is + // discovered in a link (section 13). + IssueAttachmentsNotImported = "attachments_not_imported" ) // InspectionIssue is one non-fatal finding recorded during inspection. @@ -201,6 +206,27 @@ type InspectOptions struct { // RequestedTeamName, when non-empty, is compared against the advisory bundle team values to // emit a single aggregate bundle_team_mismatch warning. The value is never used to route. RequestedTeamName string + // Now is the caller's current time in epoch milliseconds, used only to judge whether a source + // timestamp is implausibly in the future. Passing it (rather than reading the wall clock) keeps + // this package pure and deterministic in tests. When zero, a fixed year-2100 ceiling is used. + Now int64 +} + +// futureTimestampAllowance is how far past Now a source timestamp may sit before it is judged +// implausibly in the future — clock skew between the source and this server should be well under a +// day. +const futureTimestampAllowance = int64(24 * 60 * 60 * 1000) + +// year2100Millis is the fallback future ceiling used when InspectOptions.Now is not supplied. +const year2100Millis = int64(4102444800000) + +// futureCeiling returns the largest source timestamp treated as plausible: now plus a skew +// allowance when now is supplied, otherwise the fixed year-2100 ceiling. +func (o InspectOptions) futureCeiling() int64 { + if o.Now > 0 { + return o.Now + futureTimestampAllowance + } + return year2100Millis } // parsing states for the JSONL sequence. @@ -237,6 +263,13 @@ func Inspect(contents *ArchiveContents, opts InspectOptions) (*InspectionResult, return nil, inspectErr(InspectErrManifestHasErrors, "manifest reports %d producer error(s): %s", len(manifest.Errors), manifest.Errors[0]) } + // A source space key is mandatory: it becomes the ImportSource's ExternalSpaceKey, which + // ImportSource.IsValid requires. The producer omits it only when neither an organization id nor + // a space key is known — a bundle we cannot map, so reject it here rather than fail later. + if manifest.Source.SpaceKey == "" { + return nil, inspectErr(InspectErrSpaceKeyMissing, "manifest source is missing a space key") + } + res := &InspectionResult{ Manifest: manifest, JSONLSha256: contents.JSONLSha256, @@ -294,12 +327,38 @@ func parseManifest(b []byte) (*Manifest, error) { if err := dec.Decode(&m); err != nil { return nil, inspectErr(InspectErrManifestInvalid, "manifest is not valid JSON: %v", err) } + // Reject trailing data after the manifest object: a decoder stops at the first value, so without + // this a second concatenated object (or garbage) would pass silently. + if dec.More() { + return nil, inspectErr(InspectErrManifestInvalid, "manifest has trailing data after the JSON object") + } if m.Version != ManifestVersion { return nil, inspectErr(InspectErrManifestVersion, "manifest version %q is unsupported; require %q", m.Version, ManifestVersion) } return &m, nil } +// lineHasForeignPayload reports whether the line carries a payload field that does not belong to +// its declared type. The version line owns both the version and source fields; every other type +// owns exactly its matching payload. A line declaring one type but carrying another type's payload +// (e.g. {"type":"page","page":{...},"space":{...}}) is rejected so a malformed or smuggled payload +// cannot ride along unnoticed. +func lineHasForeignPayload(l *Line, declaredType string) bool { + present := map[string]bool{ + LineTypeVersion: l.Version != nil || l.Source != nil, + LineTypeSpace: l.Space != nil, + LineTypePage: l.Page != nil, + LineTypePageComment: l.PageComment != nil, + LineTypeResolveSpacePlaceholders: l.ResolveSpacePlaceholders != nil, + } + for typ, set := range present { + if typ != declaredType && set { + return true + } + } + return false +} + // parseJSONL runs the strict v2 line sequence state machine, normalizing pages into res. func parseJSONL(b []byte, manifest *Manifest, opts InspectOptions, res *InspectionResult) error { lines := splitJSONLLines(b) @@ -312,6 +371,9 @@ func parseJSONL(b []byte, manifest *Manifest, opts InspectOptions, res *Inspecti parentOf := make(map[string]string) siblingCounter := make(map[string]int) // parent external ID ("" for roots) -> next sibling ordinal teamValues := make(map[string]struct{}) + // The manifest's advisory target team is also compared against the requested team, alongside the + // per-line team values, so a mismatch declared only in the manifest is still surfaced. + collectTeam(teamValues, manifest.Target.Team) pageOrdinal := 0 @@ -327,6 +389,9 @@ func parseJSONL(b []byte, manifest *Manifest, opts InspectOptions, res *Inspecti if err := json.Unmarshal([]byte(raw), &line); err != nil { return inspectErr(InspectErrLineInvalid, "import.jsonl line %d is invalid JSON: %v", i+1, err) } + if lineHasForeignPayload(&line, line.Type) { + return inspectErr(InspectErrPayloadMismatch, "import.jsonl line %d declares type %q but carries another type's payload", i+1, line.Type) + } switch line.Type { case LineTypeVersion: @@ -366,7 +431,7 @@ func parseJSONL(b []byte, manifest *Manifest, opts InspectOptions, res *Inspecti if len(res.Pages) >= MaxPages { return inspectErr(InspectErrTooManyPages, "bundle has more than %d pages", MaxPages) } - sp, err := normalizePage(line.Page, manifest, i+1, pageOrdinal, seenPageIDs, parentOf, siblingCounter, teamValues, res) + sp, err := normalizePage(line.Page, manifest, i+1, pageOrdinal, seenPageIDs, parentOf, siblingCounter, teamValues, opts.futureCeiling(), res) if err != nil { return err } @@ -448,7 +513,7 @@ func handleSpaceLine(space *SpaceData, manifest *Manifest, teamValues map[string func normalizePage( page *PageData, manifest *Manifest, lineNo, ordinal int, seenPageIDs map[string]struct{}, parentOf map[string]string, - siblingCounter map[string]int, teamValues map[string]struct{}, res *InspectionResult, + siblingCounter map[string]int, teamValues map[string]struct{}, futureCeiling int64, res *InspectionResult, ) (*StagedPage, error) { collectTeam(teamValues, stringOrEmpty(page.Team)) @@ -491,7 +556,7 @@ func normalizePage( // Timestamp validation (does not discard the page). sourceCreateAt := int64OrZero(page.CreateAt) - if !plausibleTimestamp(sourceCreateAt) { + if !plausibleTimestamp(sourceCreateAt, futureCeiling) { res.Issues = append(res.Issues, InspectionIssue{ Severity: SeverityWarning, Code: IssueSourceCreateAtInvalid, ExternalID: externalID, Title: title, Message: "source create timestamp is missing, non-positive, or implausibly in the future", @@ -500,7 +565,7 @@ func normalizePage( }) } // update_at issue is emitted only when supplied but unusable. - if page.UpdateAt != nil && !plausibleTimestamp(*page.UpdateAt) { + if page.UpdateAt != nil && !plausibleTimestamp(*page.UpdateAt, futureCeiling) { res.Issues = append(res.Issues, InspectionIssue{ Severity: SeverityWarning, Code: IssueSourceUpdateAtInvalid, ExternalID: externalID, Title: title, Message: "source update timestamp was supplied but is not usable", @@ -536,7 +601,7 @@ func normalizePage( } if len(*page.Attachments) > 0 { res.Issues = append(res.Issues, InspectionIssue{ - Severity: SeverityInfo, Code: IssueAttachmentNotImported, ExternalID: externalID, Title: title, + Severity: SeverityInfo, Code: IssueAttachmentsNotImported, ExternalID: externalID, Title: title, Message: fmt.Sprintf("%d attachment(s) counted but not imported in this release", len(*page.Attachments)), Remediation: "Attachment import is a future release; bytes are neither extracted nor stored.", }) @@ -722,14 +787,9 @@ func emitTeamMismatch(teams map[string]struct{}, opts InspectOptions, res *Inspe } } -// plausibleTimestamp reports whether ms is a positive epoch-millis value not implausibly far in the -// future (more than ~2 days ahead of a fixed sanity ceiling is rejected). It uses no wall clock so -// the pure function stays deterministic: any positive value up to year ~2100 is accepted. -func plausibleTimestamp(ms int64) bool { - if ms <= 0 { - return false - } - // Year 2100 in epoch millis; a source date beyond this is treated as implausible. - const year2100Millis = int64(4102444800000) - return ms <= year2100Millis +// plausibleTimestamp reports whether ms is a positive epoch-millis value at or below futureCeiling +// (now + a skew allowance, or the year-2100 fallback). A non-positive or too-far-future value is +// implausible. +func plausibleTimestamp(ms, futureCeiling int64) bool { + return ms > 0 && ms <= futureCeiling } diff --git a/server/importer/inspect_test.go b/server/importer/inspect_test.go index d9bc773..7186446 100644 --- a/server/importer/inspect_test.go +++ b/server/importer/inspect_test.go @@ -7,10 +7,18 @@ import ( "archive/zip" "bytes" "errors" + "io" + "strconv" "strings" "testing" ) +// nopWriteCloser adapts an io.Writer to io.WriteCloser for a passthrough zip compressor in tests. +type nopWriteCloser struct{ w io.Writer } + +func (n nopWriteCloser) Write(p []byte) (int, error) { return n.w.Write(p) } +func (n nopWriteCloser) Close() error { return nil } + // inspectErrCode returns the stable code of an *InspectError or *ArchiveError, or "" otherwise. func inspectErrCode(err error) string { var ie *InspectError @@ -248,8 +256,8 @@ func TestInspect_CountsAndAttachments(t *testing.T) { if res.AttachmentCount != 2 { t.Errorf("attachments = %d, want 2", res.AttachmentCount) } - if !hasIssue(res, IssueAttachmentNotImported) { - t.Errorf("expected attachment_placeholder_not_imported issue") + if !hasIssue(res, IssueAttachmentsNotImported) { + t.Errorf("expected attachments_not_imported issue") } } @@ -307,6 +315,76 @@ func TestInspect_TeamMatchNoIssue(t *testing.T) { } } +func TestInspect_TeamMismatchFromManifestTargetOnly(t *testing.T) { + // Build a bundle whose per-line team values all match the requested team, but whose manifest + // target team differs. The mismatch must still be surfaced from the manifest metadata. + jsonl := joinLines( + `{"type":"version","version":2,"source":{"space_key":"DOCS"}}`, + `{"type":"space","space":{"team":"requested","title":"Docs","props":{"import_source_id":"DOCS"}}}`, + `{"type":"resolve_space_placeholders","resolve_space_placeholders":{"team":"requested","space_import_source_id":"DOCS"}}`, + ) + b := newBundle(jsonl, baseManifest(0, 0, 0)) + b.manifest.Target.Team = "manifest-team" + res, err := b.inspect(t, InspectOptions{RequestedTeamName: "requested"}) + if err != nil { + t.Fatalf("unexpected: %v", err) + } + if !hasIssue(res, IssueBundleTeamMismatch) { + t.Errorf("expected bundle_team_mismatch from manifest target team") + } +} + +func TestInspect_MissingSourceSpaceKey(t *testing.T) { + b := validBundle(t) + b.manifest.Source.SpaceKey = "" + _, err := b.inspect(t, InspectOptions{}) + if got := inspectErrCode(err); got != InspectErrSpaceKeyMissing { + t.Fatalf("code = %q, want %q", got, InspectErrSpaceKeyMissing) + } +} + +func TestInspect_MultiPayloadLineRejected(t *testing.T) { + // A line declaring type "page" but also carrying a "space" payload must be rejected. + badLine := `{"type":"page","page":{"space_import_source_id":"DOCS","user":"j","title":"H","content":` + + mustQuote(docString("x")) + `,"props":{"import_source_id":"100"}},"space":{"team":"myteam"}}` + jsonl := joinLines(versionLine(), spaceLine(), badLine, resolveLine()) + _, err := newBundle(jsonl, baseManifest(1, 0, 0)).inspect(t, InspectOptions{}) + if got := inspectErrCode(err); got != InspectErrPayloadMismatch { + t.Fatalf("code = %q, want %q", got, InspectErrPayloadMismatch) + } +} + +func TestInspect_ManifestTrailingJSON(t *testing.T) { + b := validBundle(t) + // Force a manifest body with trailing data after the object. The builder marshals the manifest, + // so instead build the archive manually via a helper that appends trailing bytes. + raw := b.bytesZipWithManifestSuffix(t, " {}") + contents, err := InspectArchive(bytes.NewReader(raw), int64(len(raw))) + if err != nil { + t.Fatalf("archive inspect failed: %v", err) + } + _, err = Inspect(contents, InspectOptions{}) + if got := inspectErrCode(err); got != InspectErrManifestInvalid { + t.Fatalf("code = %q, want %q", got, InspectErrManifestInvalid) + } +} + +func TestInspect_FutureTimestampWithNow(t *testing.T) { + // A create_at far beyond Now+allowance is implausible and must be flagged. + now := int64(1704106800000) + future := now + futureTimestampAllowance + 1_000_000 + page := `{"type":"page","page":{"space_import_source_id":"DOCS","user":"j","title":"H","content":` + + mustQuote(docString("x")) + `,"create_at":` + strconv.FormatInt(future, 10) + `,"props":{"import_source_id":"100"}}}` + jsonl := joinLines(versionLine(), spaceLine(), page, resolveLine()) + res, err := newBundle(jsonl, baseManifest(1, 0, 0)).inspect(t, InspectOptions{Now: now}) + if err != nil { + t.Fatalf("unexpected: %v", err) + } + if !hasIssue(res, IssueSourceCreateAtInvalid) { + t.Errorf("expected source_create_at_invalid for a future timestamp beyond now+allowance") + } +} + func TestInspect_SpaceKeyMismatch(t *testing.T) { // A page declaring a different space key must be rejected. badPage := `{"type":"page","page":{"space_import_source_id":"OTHER","user":"j","title":"H","content":` + @@ -472,6 +550,49 @@ func TestInspectArchive_Traversal(t *testing.T) { } } +func TestInspectArchive_DuplicateNormalizedEntry(t *testing.T) { + // "data//x" and "data/x" normalize to the same path and must be rejected as a duplicate. + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + for _, n := range []string{entryManifest, entryJSONL, "data/x", "data//x"} { + w, _ := zw.Create(n) + _, _ = w.Write([]byte("x")) + } + _ = zw.Close() + raw := buf.Bytes() + _, err := InspectArchive(bytes.NewReader(raw), int64(len(raw))) + if got := inspectErrCode(err); got != ArchiveErrDuplicateEntry { + t.Fatalf("code = %q, want %q", got, ArchiveErrDuplicateEntry) + } +} + +func TestInspectArchive_DataEntryUnsupportedMethod(t *testing.T) { + // A data/ entry using an unsupported compression method is rejected even though its bytes are + // never read. + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + for _, n := range []string{entryManifest, entryJSONL} { + w, _ := zw.Create(n) + _, _ = w.Write([]byte("x")) + } + // Method 99 is neither Store nor Deflate. Register a passthrough compressor so the writer emits + // a central-directory entry advertising method 99; the reader rejects it before opening bytes. + zw.RegisterCompressor(99, func(w io.Writer) (io.WriteCloser, error) { + return nopWriteCloser{w}, nil + }) + hw, err := zw.CreateHeader(&zip.FileHeader{Name: "data/blob.bin", Method: 99}) + if err != nil { + t.Fatalf("create header: %v", err) + } + _, _ = hw.Write([]byte("x")) + _ = zw.Close() + raw := buf.Bytes() + _, err = InspectArchive(bytes.NewReader(raw), int64(len(raw))) + if got := inspectErrCode(err); got != ArchiveErrUnsupportedMethod { + t.Fatalf("code = %q, want %q", got, ArchiveErrUnsupportedMethod) + } +} + func TestInspectArchive_DuplicateEntry(t *testing.T) { var buf bytes.Buffer zw := zip.NewWriter(&buf) diff --git a/server/importer/testhelpers_test.go b/server/importer/testhelpers_test.go index ca48f63..107f2f6 100644 --- a/server/importer/testhelpers_test.go +++ b/server/importer/testhelpers_test.go @@ -72,6 +72,37 @@ func (b *bundleBuilder) bytesZip(t *testing.T) []byte { return buf.Bytes() } +// bytesZipWithManifestSuffix builds the archive like bytesZip but appends suffix to the manifest +// entry's bytes after its JSON object, for exercising trailing-data rejection. +func (b *bundleBuilder) bytesZipWithManifestSuffix(t *testing.T, suffix string) []byte { + t.Helper() + m := b.manifest + if !b.skipChecksum && m.Checksums.JSONLSha256 == "" { + m.Checksums.JSONLSha256 = b.jsonlSha() + } + manifestBytes, err := json.Marshal(m) + if err != nil { + t.Fatalf("marshal manifest: %v", err) + } + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + writeEntry := func(name, body string) { + w, cerr := zw.Create(name) + if cerr != nil { + t.Fatalf("zip create %q: %v", name, cerr) + } + if _, werr := w.Write([]byte(body)); werr != nil { + t.Fatalf("zip write %q: %v", name, werr) + } + } + writeEntry(entryManifest, string(manifestBytes)+suffix) + writeEntry(entryJSONL, b.jsonl) + if err := zw.Close(); err != nil { + t.Fatalf("zip close: %v", err) + } + return buf.Bytes() +} + // inspectBundle runs InspectArchive + Inspect over the builder's bytes. func (b *bundleBuilder) inspect(t *testing.T, opts InspectOptions) (*InspectionResult, error) { t.Helper() diff --git a/server/model/import.go b/server/model/import.go index 4a4bb30..02d39ba 100644 --- a/server/model/import.go +++ b/server/model/import.go @@ -12,7 +12,7 @@ import ( ) // ImportJobState is a persisted import-job lifecycle state. The values here must exactly match the -// chk_docs_importjob_state CHECK constraint in migration 000005. +// chk_docs_importjob_state CHECK constraint in migration 000006. type ImportJobState string const ( @@ -412,7 +412,9 @@ func (j *ImportJob) IsValid() *mmmodel.AppError { if !j.State.IsValid() { return mmmodel.NewAppError(where, "model.import_job.is_valid.state.app_error", nil, "id="+j.Id, http.StatusBadRequest) } - if j.BundleSha256 != "" && !hexSHA256.MatchString(j.BundleSha256) { + // BundleSha256 is always computed during upload inspection, so a persisted job must carry a + // valid 64-hex digest; an empty value is a bug, not a valid pre-inspection state. + if !hexSHA256.MatchString(j.BundleSha256) { return mmmodel.NewAppError(where, "model.import_job.is_valid.bundle_sha.app_error", nil, "id="+j.Id, http.StatusBadRequest) } if !IsValidImportHash(j.PreflightRevision) { diff --git a/server/model/import_test.go b/server/model/import_test.go index 46282ae..2276453 100644 --- a/server/model/import_test.go +++ b/server/model/import_test.go @@ -78,6 +78,7 @@ func TestImportJob_IsValid(t *testing.T) { "bad source mode": func(j *ImportJob) { j.SourceSelectionMode = "maybe" }, "bad state": func(j *ImportJob) { j.State = "limbo" }, "bad bundle sha": func(j *ImportJob) { j.BundleSha256 = "nothex" }, + "empty bundle sha": func(j *ImportJob) { j.BundleSha256 = "" }, "bad preflight rev": func(j *ImportJob) { j.PreflightRevision = "short" }, "zero timestamps": func(j *ImportJob) { j.CreateAt = 0 }, "long space title": func(j *ImportJob) { j.ConfirmedSpaceTitle = strings.Repeat("x", ImportSpaceTitleMaxRunes+1) }, diff --git a/server/store/migrations/000005_create_imports.down.sql b/server/store/migrations/000006_create_imports.down.sql similarity index 100% rename from server/store/migrations/000005_create_imports.down.sql rename to server/store/migrations/000006_create_imports.down.sql diff --git a/server/store/migrations/000005_create_imports.up.sql b/server/store/migrations/000006_create_imports.up.sql similarity index 100% rename from server/store/migrations/000005_create_imports.up.sql rename to server/store/migrations/000006_create_imports.up.sql From b19470ce5b91e7a2fad5731c0a37a497e0c2fb33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20Vay=C3=A1?= Date: Mon, 27 Jul 2026 13:36:08 +0200 Subject: [PATCH 4/5] Confluence import: fix six review findings (trailing JSON, revision, 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) --- server/importer/archive.go | 12 +++-- server/importer/inspect.go | 51 +++++++++++++-------- server/importer/inspect_test.go | 78 ++++++++++++++++++++++++++++++--- server/importer/links.go | 28 +++++++++--- server/importer/tiptap.go | 9 +++- server/importer/tiptap_test.go | 52 ++++++++++++++++++++++ server/model/import_report.go | 5 +++ 7 files changed, 198 insertions(+), 37 deletions(-) diff --git a/server/importer/archive.go b/server/importer/archive.go index 2b095ad..246c5f0 100644 --- a/server/importer/archive.go +++ b/server/importer/archive.go @@ -141,9 +141,15 @@ func InspectArchive(r io.ReaderAt, n int64) (*ArchiveContents, error) { } normSeen[name] = struct{}{} - // Validate mode, encryption, and compression method for every file entry — including the - // data/ payloads we never open — so an unsafe entry is rejected uniformly rather than only - // for the two entries whose bytes are read. + // Reject symlinks for every entry, including directory-named ones: a symlink whose name ends + // in "/" (e.g. "data/") would otherwise skip the file checks below and be silently accepted. + if f.Mode()&fsModeSymlink != 0 { + return nil, archiveErr(ArchiveErrUnsafeEntry, "archive entry %q is a symlink", raw) + } + // Validate the remaining mode, encryption, and compression constraints for every file entry + // — including the data/ payloads we never open — so an unsafe entry is rejected uniformly + // rather than only for the two entries whose bytes are read. Directory entries are exempt: + // they are legitimately non-regular and are never opened. isDir := strings.HasSuffix(name, "/") if !isDir { if modeErr := checkEntryMode(f); modeErr != nil { diff --git a/server/importer/inspect.go b/server/importer/inspect.go index 0551a99..c07ff7b 100644 --- a/server/importer/inspect.go +++ b/server/importer/inspect.go @@ -5,10 +5,14 @@ package importer import ( "encoding/json" + "errors" "fmt" + "io" "strings" "unicode/utf8" + mmmodel "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost-plugin-docs/server/model" ) @@ -114,6 +118,7 @@ const ( InspectErrSpaceKeyMissing = "space_key_missing" InspectErrCommentMissingPageID = "comment_missing_page_id" InspectErrAttachmentPath = "attachment_invalid_path" + InspectErrCountMismatch = "manifest_count_mismatch" InspectErrHash = "hash_failed" ) @@ -131,7 +136,6 @@ const ( IssueAttachmentChecksumNotVerified = "attachment_checksum_not_verified" IssueSourceCreateAtInvalid = "source_create_at_invalid" IssueSourceUpdateAtInvalid = "source_update_at_invalid" - IssueManifestCountMismatch = "manifest_count_mismatch" IssuePlaceholderInText = "placeholder_in_text_not_rewritten" // IssueAttachmentsNotImported flags a page that carries attachment records, none of which are // imported in this release. This is the plan's partial-scope code (section 20.2), distinct from @@ -314,7 +318,9 @@ func Inspect(contents *ArchiveContents, opts InspectOptions) (*InspectionResult, return nil, err } - reconcileCounts(manifest, res) + if err := reconcileCounts(manifest, res); err != nil { + return nil, err + } summarizeRestricted(manifest, res) return res, nil @@ -327,9 +333,10 @@ func parseManifest(b []byte) (*Manifest, error) { if err := dec.Decode(&m); err != nil { return nil, inspectErr(InspectErrManifestInvalid, "manifest is not valid JSON: %v", err) } - // Reject trailing data after the manifest object: a decoder stops at the first value, so without - // this a second concatenated object (or garbage) would pass silently. - if dec.More() { + // Reject trailing data after the manifest object. json.Decoder.More() is not a trailing-data + // check — it returns false before a closing "]"/"}" delimiter, so "{...}]" would pass. Decoding + // a second value and requiring io.EOF rejects any trailing token while tolerating whitespace. + if err := dec.Decode(&struct{}{}); !errors.Is(err, io.EOF) { return nil, inspectErr(InspectErrManifestInvalid, "manifest has trailing data after the JSON object") } if m.Version != ManifestVersion { @@ -526,7 +533,11 @@ func normalizePage( return nil, inspectErr(InspectErrDuplicatePageID, "line %d: duplicate page external id %q", lineNo, externalID) } - title := strings.TrimSpace(stringOrEmpty(page.Title)) + // Normalize the title exactly as Page.PreSave does (SanitizeUnicode then trim). Import uses a + // dedicated store path that bypasses PreSave, so without this the stored title could carry + // unsafe Unicode controls, and — more subtly — the incoming source hash would be computed on a + // value that never matches the applied hash of the normalized title, causing spurious conflicts. + title := strings.TrimSpace(mmmodel.SanitizeUnicode(stringOrEmpty(page.Title))) if title == "" { return nil, inspectErr(InspectErrPageMissingTitle, "line %d: page %q is missing a title", lineNo, externalID) } @@ -718,21 +729,25 @@ func validateAttachmentPath(p, externalID string, lineNo int) error { return nil } -// reconcileCounts compares parsed counts against the manifest and warns on any mismatch. -func reconcileCounts(manifest *Manifest, res *InspectionResult) { - check := func(name string, parsed, declared int) { +// reconcileCounts rejects the bundle when a parsed entity count disagrees with the manifest. The +// JSONL checksum is already verified, and the producer writes exactly one page/comment line per +// counted entity (and one attachment count per emitted attachment), so for any well-formed bundle +// the manifest counts equal the parsed counts exactly. A mismatch therefore signals a corrupt or +// internally inconsistent producer bundle, which must be rejected rather than imported partially. +func reconcileCounts(manifest *Manifest, res *InspectionResult) error { + check := func(name string, parsed, declared int) error { if declared != parsed { - res.Issues = append(res.Issues, InspectionIssue{ - Severity: SeverityWarning, Code: IssueManifestCountMismatch, - Message: fmt.Sprintf("manifest declares %d %s but %d were parsed", declared, name, parsed), - Remediation: "The parsed counts are authoritative; the manifest may be stale.", - Details: map[string]any{"entity": name, "declared": declared, "parsed": parsed}, - }) + return inspectErr(InspectErrCountMismatch, "manifest declares %d %s but %d were parsed", declared, name, parsed) } + return nil + } + if err := check("pages", len(res.Pages), manifest.Counts.Pages); err != nil { + return err + } + if err := check("comments", res.CommentCount, manifest.Counts.Comments); err != nil { + return err } - check("pages", len(res.Pages), manifest.Counts.Pages) - check("comments", res.CommentCount, manifest.Counts.Comments) - check("attachments", res.AttachmentCount, manifest.Counts.Attachments) + return check("attachments", res.AttachmentCount, manifest.Counts.Attachments) } // summarizeRestricted intersects the manifest restricted list with emitted (staged) page IDs. diff --git a/server/importer/inspect_test.go b/server/importer/inspect_test.go index 7186446..4a6fe95 100644 --- a/server/importer/inspect_test.go +++ b/server/importer/inspect_test.go @@ -8,9 +8,12 @@ import ( "bytes" "errors" "io" + "io/fs" "strconv" "strings" "testing" + + mmmodel "github.com/mattermost/mattermost/server/public/model" ) // nopWriteCloser adapts an io.Writer to io.WriteCloser for a passthrough zip compressor in tests. @@ -262,15 +265,14 @@ func TestInspect_CountsAndAttachments(t *testing.T) { } func TestInspect_ManifestCountMismatch(t *testing.T) { - res, err := newBundle( + // A checksum-valid JSONL with one page but a manifest declaring five is a corrupt/inconsistent + // producer bundle and must be rejected, not merely warned about. + _, err := newBundle( joinLines(versionLine(), spaceLine(), pageLine(t, "100", "", "H", docString("x")), resolveLine()), baseManifest(5, 0, 0), // declares 5 pages, but only 1 parsed ).inspect(t, InspectOptions{}) - if err != nil { - t.Fatalf("unexpected: %v", err) - } - if !hasIssue(res, IssueManifestCountMismatch) { - t.Errorf("expected manifest_count_mismatch issue") + if got := inspectErrCode(err); got != InspectErrCountMismatch { + t.Fatalf("code = %q, want %q", got, InspectErrCountMismatch) } } @@ -358,7 +360,9 @@ func TestInspect_ManifestTrailingJSON(t *testing.T) { b := validBundle(t) // Force a manifest body with trailing data after the object. The builder marshals the manifest, // so instead build the archive manually via a helper that appends trailing bytes. - raw := b.bytesZipWithManifestSuffix(t, " {}") + // A trailing "]" after the manifest object is the case json.Decoder.More() misses (it returns + // false before a closing delimiter); the io.EOF check must still reject it. + raw := b.bytesZipWithManifestSuffix(t, "]") contents, err := InspectArchive(bytes.NewReader(raw), int64(len(raw))) if err != nil { t.Fatalf("archive inspect failed: %v", err) @@ -438,6 +442,42 @@ func TestInspect_ManifestWarningsCapped(t *testing.T) { } } +func TestInspect_TitleSanitizedLikePreSave(t *testing.T) { + // U+202E (a BIDI control) is stripped by mmmodel.SanitizeUnicode, matching Page.PreSave. The + // staged title must be sanitized, and the incoming source hash computed on the sanitized value. + // Build the RIGHT-TO-LEFT OVERRIDE (U+202E) from its code point so no literal BIDI control sits + // in this source file (which would trip bidichk/gosec). + rlo := string(rune(0x202e)) + rawTitle := "Hi" + rlo + "There" + page := `{"type":"page","page":{"space_import_source_id":"DOCS","user":"j","title":` + + mustQuote(rawTitle) + `,"content":` + mustQuote(docString("x")) + `,"props":{"import_source_id":"100"}}}` + jsonl := joinLines(versionLine(), spaceLine(), page, resolveLine()) + res, err := newBundle(jsonl, baseManifest(1, 0, 0)).inspect(t, InspectOptions{}) + if err != nil { + t.Fatalf("unexpected: %v", err) + } + want := mmmodel.SanitizeUnicode(rawTitle) + if res.Pages[0].Title != want { + t.Fatalf("staged title = %q, want sanitized %q", res.Pages[0].Title, want) + } + if strings.Contains(res.Pages[0].Title, rlo) { + t.Errorf("staged title still contains the stripped control rune") + } + // The hash must be built on the sanitized title, so it equals a recompute using that title. + wantHash, herr := HashSourceState(SourceStateHashInput{ + Title: want, + CanonicalBody: res.Pages[0].CanonicalBody, + AuthorProposal: "j", + SourceProps: map[string]any{}, + }) + if herr != nil { + t.Fatalf("hash: %v", herr) + } + if res.Pages[0].IncomingSourceHash != wantHash { + t.Errorf("incoming hash not based on sanitized title") + } +} + func TestInspect_InvalidTipTap(t *testing.T) { jsonl := joinLines(versionLine(), spaceLine(), pageLine(t, "100", "", "H", `{"type":"notdoc"}`), resolveLine()) @@ -593,6 +633,30 @@ func TestInspectArchive_DataEntryUnsupportedMethod(t *testing.T) { } } +func TestInspectArchive_SymlinkDirEntryRejected(t *testing.T) { + // A symlink whose name ends in "/" must not slip past the file checks by looking like a + // directory. + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + for _, n := range []string{entryManifest, entryJSONL} { + w, _ := zw.Create(n) + _, _ = w.Write([]byte("x")) + } + hdr := &zip.FileHeader{Name: "data/", Method: zip.Store} + hdr.SetMode(fs.ModeSymlink | 0o777) + hw, err := zw.CreateHeader(hdr) + if err != nil { + t.Fatalf("create header: %v", err) + } + _, _ = hw.Write([]byte("/etc")) + _ = zw.Close() + raw := buf.Bytes() + _, err = InspectArchive(bytes.NewReader(raw), int64(len(raw))) + if got := inspectErrCode(err); got != ArchiveErrUnsafeEntry { + t.Fatalf("code = %q, want %q", got, ArchiveErrUnsafeEntry) + } +} + func TestInspectArchive_DuplicateEntry(t *testing.T) { var buf bytes.Buffer zw := zip.NewWriter(&buf) diff --git a/server/importer/links.go b/server/importer/links.go index 6ea91ec..e98459f 100644 --- a/server/importer/links.go +++ b/server/importer/links.go @@ -3,7 +3,10 @@ package importer -import "regexp" +import ( + "regexp" + "strings" +) // Confluence link placeholder names the mmetl producer emits inside TipTap link mark hrefs and // image src attributes. The producer wraps them in double braces, e.g. "{{CONF_PAGE_ID:101}}" @@ -49,19 +52,30 @@ var placeholderKinds = map[string]LinkKind{ PlaceholderAttachment: LinkKindAttachment, } +// placeholderTarget matches a placeholder's target argument: any run of escaped braces ("\{" or +// "\}") or characters that are neither brace. The producer escapes literal braces in the target +// (escapeForPlaceholder: "{"->"\{", "}"->"\}"), so a naive "[^}]*" would stop at the first escaped +// "}" and fail to match a title/filename containing braces. RE2 has no lookbehind, so the "stop at +// the first unescaped }}" rule is expressed through this alternation instead. +const placeholderTarget = `((?:\\[{}]|[^{}])*)` + // linkPlaceholderRe matches a producer link/attachment placeholder of the form -// "{{CONF_PAGE_ID:target}}", capturing the placeholder name and its target argument. The producer -// URL-escapes the target, so it never itself contains "}". -var linkPlaceholderRe = regexp.MustCompile(`\{\{(CONF_PAGE_ID|CONF_PAGE_TITLE|CONF_FILE|CONF_ATTACHMENT):([^}]*)\}\}`) +// "{{CONF_PAGE_ID:target}}", capturing the placeholder name and its (still-escaped) target. +var linkPlaceholderRe = regexp.MustCompile(`\{\{(CONF_PAGE_ID|CONF_PAGE_TITLE|CONF_FILE|CONF_ATTACHMENT):` + placeholderTarget + `\}\}`) // anyPlaceholderRe matches any Confluence placeholder token (including e.g. CONF_USER) so a // placeholder left in ordinary text can be flagged even when it is not one of the link kinds. -var anyPlaceholderRe = regexp.MustCompile(`\{\{CONF_[A-Z_]+:[^}]*\}\}`) +var anyPlaceholderRe = regexp.MustCompile(`\{\{CONF_[A-Z_]+:` + placeholderTarget + `\}\}`) + +// placeholderUnescaper reverses escapeForPlaceholder, turning the producer's "\{"/"\}" back into +// literal braces in a discovered target. +var placeholderUnescaper = strings.NewReplacer(`\{`, `{`, `\}`, `}`) // classifyPlaceholder inspects an approved attribute value (a link href or image src) and returns a // DiscoveredLink when it contains a recognized "{{CONF_...:target}}" placeholder, plus ok=true. The // producer sets the whole attribute to the placeholder, but this tolerates surrounding text by -// matching the first placeholder anywhere in the value. +// matching the first placeholder anywhere in the value. The captured target is unescaped back to +// its literal form. func classifyPlaceholder(value string, inImageSrc bool) (DiscoveredLink, bool) { m := linkPlaceholderRe.FindStringSubmatch(value) if m == nil { @@ -70,7 +84,7 @@ func classifyPlaceholder(value string, inImageSrc bool) (DiscoveredLink, bool) { return DiscoveredLink{ Kind: placeholderKinds[m[1]], Raw: value, - Target: m[2], + Target: placeholderUnescaper.Replace(m[2]), InImageSrc: inImageSrc, }, true } diff --git a/server/importer/tiptap.go b/server/importer/tiptap.go index c4b0440..d72f9ed 100644 --- a/server/importer/tiptap.go +++ b/server/importer/tiptap.go @@ -6,7 +6,9 @@ package importer import ( "bytes" "encoding/json" + "errors" "fmt" + "io" "regexp" "strings" @@ -75,8 +77,11 @@ func CanonicalizeAndExtractSearchText(body string) (canonicalBody string, search if decErr := dec.Decode(&root); decErr != nil { return "", "", nil, tiptapErr(TipTapErrInvalidJSON, "content is not valid JSON: %v", decErr) } - // Reject trailing data after the first JSON value. - if dec.More() { + // Reject trailing data after the first JSON value. json.Decoder.More() cannot be used here: it + // returns false before a closing "]"/"}" delimiter, so "{...}]" would slip through. Decoding a + // second value and requiring io.EOF rejects any trailing token while still tolerating trailing + // whitespace. + if decErr := dec.Decode(&struct{}{}); !errors.Is(decErr, io.EOF) { return "", "", nil, tiptapErr(TipTapErrInvalidJSON, "content has trailing data after the root JSON value") } diff --git a/server/importer/tiptap_test.go b/server/importer/tiptap_test.go index 211f34c..6eb21ba 100644 --- a/server/importer/tiptap_test.go +++ b/server/importer/tiptap_test.go @@ -17,6 +17,20 @@ func TestCanonicalize_RejectsNonDoc(t *testing.T) { } } +func TestCanonicalize_RejectsTrailingDelimiter(t *testing.T) { + // json.Decoder.More() returns false before a closing "]"/"}", so these must be caught by the + // decode-second-value/io.EOF check rather than More(). + for _, body := range []string{`{"type":"doc","content":[]}]`, `{"type":"doc","content":[]}}`, `{"type":"doc"}{}`} { + if _, _, _, err := CanonicalizeAndExtractSearchText(body); err == nil { + t.Errorf("expected trailing-data rejection for %q", body) + } + } + // Trailing whitespace remains acceptable. + if _, _, _, err := CanonicalizeAndExtractSearchText(`{"type":"doc","content":[]}` + " \n"); err != nil { + t.Errorf("trailing whitespace should be accepted: %v", err) + } +} + func TestCanonicalize_PreservesUnknownTypes(t *testing.T) { body := `{"type":"doc","content":[{"type":"customWidget","attrs":{"foo":"bar"},"content":[{"type":"text","text":"hi"}]}]}` canon, search, _, err := CanonicalizeAndExtractSearchText(body) @@ -179,6 +193,44 @@ func TestLinkDiscovery_OnlyApprovedAttrs(t *testing.T) { } } +func TestLinkDiscovery_EscapedBracesInTarget(t *testing.T) { + // The producer escapes literal braces in a placeholder target ("{"->"\{", "}"->"\}"). A title + // containing braces must still be discovered, and its target unescaped back to literal form. + doc := map[string]any{ + "type": "doc", + "content": []any{ + map[string]any{ + "type": "paragraph", + "content": []any{ + map[string]any{ + "type": "text", + "text": "linky", + "marks": []any{map[string]any{"type": "link", "attrs": map[string]any{ + "href": `{{CONF_PAGE_TITLE:A\{B\}C}}`, + }}}, + }, + }, + }, + }, + } + _, _, links, err := CanonicalizeAndExtractSearchText(marshal(doc)) + if err != nil { + t.Fatalf("unexpected: %v", err) + } + var found *DiscoveredLink + for i := range links { + if links[i].Kind == LinkKindPageTitle { + found = &links[i] + } + } + if found == nil { + t.Fatalf("expected a page-title placeholder to be discovered; got %+v", links) + } + if found.Target != "A{B}C" { + t.Errorf("target = %q, want unescaped %q", found.Target, "A{B}C") + } +} + // --- helpers --- func block(kind string, children ...any) map[string]any { diff --git a/server/model/import_report.go b/server/model/import_report.go index 03bd74f..e2b8cd2 100644 --- a/server/model/import_report.go +++ b/server/model/import_report.go @@ -100,6 +100,11 @@ type ImportReportSummary struct { GeneratedAt int64 `json:"generated_at"` Fidelity ImportFidelity `json:"fidelity"` Counts ImportReportCounts `json:"counts"` + // Revision is the canonical preflight revision (a SHA-256 digest) the client must echo back in + // its confirmation request. It is populated only on the preflight summary; the final summary + // leaves it empty. Exposing it here lets a client build a valid confirmation without access to + // the internal job model. + Revision string `json:"revision,omitempty"` } // ImportReport is the full downloadable report. Results and Issues stream from persisted rows so From 5eab70610a3faed729138e572545397daa235f8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20Vay=C3=A1?= Date: Mon, 27 Jul 2026 17:22:07 +0200 Subject: [PATCH 5/5] Confluence import: fix confirmation size cap and NUL-in-content persistence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- server/importer/contract.go | 37 +++++++++++++++++++ server/importer/inspect.go | 26 ++++++++------ server/importer/inspect_test.go | 59 ++++++++++++++++++++++++++++++ server/importer/tiptap.go | 3 ++ server/model/import.go | 64 ++++++++++++++++++++++++++++++++- server/model/import_test.go | 44 +++++++++++++++++++++++ 6 files changed, 222 insertions(+), 11 deletions(-) diff --git a/server/importer/contract.go b/server/importer/contract.go index daf422e..d32fddc 100644 --- a/server/importer/contract.go +++ b/server/importer/contract.go @@ -8,6 +8,8 @@ // in isolation; the app/store layers orchestrate it. package importer +import "strings" + // ContractVersion is the only JSONL contract version this importer accepts. const ContractVersion = 2 @@ -143,3 +145,38 @@ func derefProps(p *map[string]any) map[string]any { } return *p } + +// stripNUL removes NUL (U+0000) bytes from s. PostgreSQL cannot store a NUL in a TEXT/VARCHAR value +// and rejects the escaped-NUL code point inside a JSONB string, so any NUL in producer content +// would fail the staging insert with an opaque DB error even though the bundle inspected cleanly. +// Dropping it during inspection keeps an otherwise valid bundle importable — mirroring how +// mmmodel.SanitizeUnicode silently drops other disallowed runes — and, because it runs before +// hashing, keeps the source hash consistent with the value actually stored. +func stripNUL(s string) string { + if !strings.ContainsRune(s, 0) { + return s + } + return strings.ReplaceAll(s, "\x00", "") +} + +// stripNULFromValue recursively removes NUL bytes from every string in a decoded JSON value +// (strings, array elements, and nested object values), so a NUL cannot survive inside a +// JSONB-persisted source-props map. It mutates maps/slices in place and returns v for convenience. +func stripNULFromValue(v any) any { + switch t := v.(type) { + case string: + return stripNUL(t) + case []any: + for i := range t { + t[i] = stripNULFromValue(t[i]) + } + return t + case map[string]any: + for k, val := range t { + t[k] = stripNULFromValue(val) + } + return t + default: + return v + } +} diff --git a/server/importer/inspect.go b/server/importer/inspect.go index c07ff7b..3d87749 100644 --- a/server/importer/inspect.go +++ b/server/importer/inspect.go @@ -533,11 +533,12 @@ func normalizePage( return nil, inspectErr(InspectErrDuplicatePageID, "line %d: duplicate page external id %q", lineNo, externalID) } - // Normalize the title exactly as Page.PreSave does (SanitizeUnicode then trim). Import uses a - // dedicated store path that bypasses PreSave, so without this the stored title could carry - // unsafe Unicode controls, and — more subtly — the incoming source hash would be computed on a - // value that never matches the applied hash of the normalized title, causing spurious conflicts. - title := strings.TrimSpace(mmmodel.SanitizeUnicode(stringOrEmpty(page.Title))) + // Normalize the title exactly as Page.PreSave does (SanitizeUnicode then trim), plus stripNUL + // (SanitizeUnicode does not drop NUL, which PostgreSQL cannot store). Import uses a dedicated + // store path that bypasses PreSave, so without this the stored title could carry unsafe Unicode + // controls or a NUL, and — more subtly — the incoming source hash would be computed on a value + // that never matches the applied hash of the normalized title, causing spurious conflicts. + title := strings.TrimSpace(stripNUL(mmmodel.SanitizeUnicode(stringOrEmpty(page.Title)))) if title == "" { return nil, inspectErr(InspectErrPageMissingTitle, "line %d: page %q is missing a title", lineNo, externalID) } @@ -585,14 +586,19 @@ func normalizePage( }) } - sourceProps := allowlistSourceProps(props) + // Strip NUL from every persisted free-text field (SanitizeUnicode does not remove it) so a NUL + // in producer content cannot break the TEXT/JSONB staging insert. This runs before hashing so + // the incoming hash matches the value actually stored. + sourceProps, _ := stripNULFromValue(allowlistSourceProps(props)).(map[string]any) + authorAccountID := stripNUL(propString(props, PropConfluenceAuthorAccountID)) + userProposal := stripNUL(stringOrEmpty(page.User)) incomingHash, hErr := HashSourceState(SourceStateHashInput{ Title: title, CanonicalBody: canonicalBody, ParentExternalID: parentID, - AuthorAccountID: propString(props, PropConfluenceAuthorAccountID), - AuthorProposal: stringOrEmpty(page.User), + AuthorAccountID: authorAccountID, + AuthorProposal: userProposal, SourceCreateAt: sourceCreateAt, SourceUpdateAt: int64OrZero(page.UpdateAt), SourceProps: sourceProps, @@ -644,8 +650,8 @@ func normalizePage( Title: title, CanonicalBody: canonicalBody, SearchText: searchText, - SourceUserProposal: stringOrEmpty(page.User), - SourceAuthorAccountID: propString(props, PropConfluenceAuthorAccountID), + SourceUserProposal: userProposal, + SourceAuthorAccountID: authorAccountID, SourceCreateAt: sourceCreateAt, SourceUpdateAt: int64OrZero(page.UpdateAt), SourceProps: sourceProps, diff --git a/server/importer/inspect_test.go b/server/importer/inspect_test.go index 4a6fe95..e1668b6 100644 --- a/server/importer/inspect_test.go +++ b/server/importer/inspect_test.go @@ -6,6 +6,7 @@ package importer import ( "archive/zip" "bytes" + "encoding/json" "errors" "io" "io/fs" @@ -478,6 +479,64 @@ func TestInspect_TitleSanitizedLikePreSave(t *testing.T) { } } +func TestInspect_StripsNULFromPersistedFields(t *testing.T) { + // A NUL (U+0000) in the title, TipTap text, and an import_labels prop must be stripped during + // inspection so the staging insert into TEXT/JSONB columns cannot fail on an unstorable byte. + // The producer emits NUL as a JSON \u escape, which json.Unmarshal decodes to a real NUL byte — + // build the line via json.Marshal so the escaping is exactly what a real bundle carries. + nul := string(rune(0)) + docJSON := `{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":` + + mustQuote("hel"+nul+"lo") + `}]}]}` + pageObj := map[string]any{ + "type": "page", + "page": map[string]any{ + "space_import_source_id": "DOCS", + "user": "j" + nul + "doe", + "title": "Ti" + nul + "tle", + "content": docJSON, + "props": map[string]any{ + "import_source_id": "100", + "confluence_author_account_id": "aa" + nul + "id", + "import_labels": []any{"la" + nul + "bel"}, + }, + }, + } + pageBytes, err := json.Marshal(pageObj) + if err != nil { + t.Fatalf("marshal page: %v", err) + } + jsonl := joinLines(versionLine(), spaceLine(), string(pageBytes), resolveLine()) + res, err := newBundle(jsonl, baseManifest(1, 0, 0)).inspect(t, InspectOptions{}) + if err != nil { + t.Fatalf("unexpected: %v", err) + } + p := res.Pages[0] + fields := map[string]string{ + "title": p.Title, + "search_text": p.SearchText, + "user_proposal": p.SourceUserProposal, + "author_id": p.SourceAuthorAccountID, + } + for name, v := range fields { + if strings.ContainsRune(v, 0) { + t.Errorf("%s still contains a NUL: %q", name, v) + } + } + if labels, ok := p.SourceProps["import_labels"].([]any); ok { + for _, l := range labels { + if s, _ := l.(string); strings.ContainsRune(s, 0) { + t.Errorf("import_labels entry still contains a NUL: %q", s) + } + } + } else { + t.Errorf("expected import_labels in source props, got %+v", p.SourceProps) + } + // Sanity: the surrounding characters survived (only the NUL was removed). + if p.Title != "Title" || p.SourceUserProposal != "jdoe" { + t.Errorf("stripping removed more than the NUL: title=%q user=%q", p.Title, p.SourceUserProposal) + } +} + func TestInspect_InvalidTipTap(t *testing.T) { jsonl := joinLines(versionLine(), spaceLine(), pageLine(t, "100", "", "H", `{"type":"notdoc"}`), resolveLine()) diff --git a/server/importer/tiptap.go b/server/importer/tiptap.go index d72f9ed..bb53b2b 100644 --- a/server/importer/tiptap.go +++ b/server/importer/tiptap.go @@ -243,6 +243,9 @@ var ( // normalizeSearchText collapses horizontal whitespace and excess blank lines, then trims. func normalizeSearchText(s string) string { + // Drop NUL bytes first: a TipTap text node may decode an escaped NUL to a literal NUL byte, + // which PostgreSQL cannot store in the SearchText TEXT column and would reject at staging insert. + s = stripNUL(s) // Normalize CRLF/CR to LF first so newline collapsing is uniform. s = strings.ReplaceAll(s, "\r\n", "\n") s = strings.ReplaceAll(s, "\r", "\n") diff --git a/server/model/import.go b/server/model/import.go index 02d39ba..56409b0 100644 --- a/server/model/import.go +++ b/server/model/import.go @@ -4,6 +4,9 @@ package model import ( + "database/sql/driver" + "encoding/json" + "fmt" "net/http" "regexp" "unicode/utf8" @@ -151,6 +154,62 @@ const ( ImportIssueCodeMaxRunes = 64 ) +// ImportConfirmationMaxBytes bounds the persisted confirmation JSON (see ImportConfirmation). It is +// set well above the realistic worst case: the confirmation carries one overwrite descriptor per +// approved conflict, each ~350 bytes (three 64-char baseline hashes + external id + timestamp), so +// a reimport approving the plan's ceiling of 5,000 conflicting pages produces ~1.75 MiB. 4 MiB +// leaves ample headroom while still bounding an abusive payload. +const ImportConfirmationMaxBytes = 4 * 1024 * 1024 + +// ImportConfirmation is the persisted confirmation payload (plan §17), stored in the +// DOCS_ImportJob.Confirmation JSONB column as raw JSON. +// +// It deliberately is NOT mmmodel.StringInterface. That type's database Value() rejects any +// marshaled JSON larger than its internal maxPropSizeBytes (1 MiB), but a *valid* confirmation can +// legitimately exceed 1 MiB: with up to 5,000 individually approved conflict overwrite descriptors +// (~350 bytes each) the payload approaches ~1.75 MiB, so StringInterface would make a legitimate +// confirmation impossible to persist (ErrMaxPropSizeExceeded at insert). ImportConfirmation instead +// stores the raw JSON bytes verbatim and enforces its own, deliberately higher bound +// (ImportConfirmationMaxBytes). The bundle summaries keep using StringInterface because they hold +// fixed-shape count structures that never approach 1 MiB; only the confirmation grows with input. +// +// The Phase-4 confirm HTTP handler must cap its request body to ImportConfirmationMaxBytes so an +// over-limit confirmation is rejected as a 413 at the edge rather than failing opaquely at insert; +// the Value() bound below is the last-line backstop, not the primary gate. +type ImportConfirmation json.RawMessage + +// Value implements driver.Valuer for the JSONB column. An empty confirmation persists as "{}" so +// the column's NOT NULL DEFAULT '{}' invariant holds, and an over-limit payload is rejected here as +// a backstop. The raw bytes are returned as a string, which lib/pq sends to a jsonb column. +func (c ImportConfirmation) Value() (driver.Value, error) { + if len(c) == 0 { + return "{}", nil + } + if len(c) > ImportConfirmationMaxBytes { + return nil, fmt.Errorf("import confirmation of %d bytes exceeds the %d byte limit", len(c), ImportConfirmationMaxBytes) + } + return string(c), nil +} + +// Scan implements sql.Scanner, reading the JSONB column back as raw JSON bytes (lib/pq yields +// []byte for jsonb; a string form is also accepted defensively). The bytes are copied so the +// value does not alias a driver-owned buffer. +func (c *ImportConfirmation) Scan(src any) error { + switch v := src.(type) { + case nil: + *c = nil + case []byte: + b := make([]byte, len(v)) + copy(b, v) + *c = b + case string: + *c = ImportConfirmation(v) + default: + return fmt.Errorf("unsupported Scan type %T for ImportConfirmation", src) + } + return nil +} + // hexSHA256 matches exactly 64 lowercase hexadecimal characters. Every non-empty SHA-256 column is // validated against this at the model/application boundary so a malformed or CHAR-padded value // never enters a comparison. @@ -207,7 +266,7 @@ type ImportJob struct { BundleSummary mmmodel.StringInterface `json:"bundle_summary"` PreflightSummary mmmodel.StringInterface `json:"preflight_summary"` PreflightRevision string `json:"preflight_revision,omitempty"` - Confirmation mmmodel.StringInterface `json:"-"` + Confirmation ImportConfirmation `json:"-"` FinalSummary mmmodel.StringInterface `json:"final_summary"` ErrorCode string `json:"error_code,omitempty"` @@ -431,6 +490,9 @@ func (j *ImportJob) IsValid() *mmmodel.AppError { if utf8.RuneCountInString(j.ErrorCode) > ImportErrorCodeMaxRunes { return mmmodel.NewAppError(where, "model.import_job.is_valid.error_code_length.app_error", map[string]any{"MaxLength": ImportErrorCodeMaxRunes}, "id="+j.Id, http.StatusBadRequest) } + if len(j.Confirmation) > ImportConfirmationMaxBytes { + return mmmodel.NewAppError(where, "model.import_job.is_valid.confirmation_too_large.app_error", map[string]any{"MaxBytes": ImportConfirmationMaxBytes}, "id="+j.Id, http.StatusBadRequest) + } if j.CreateAt == 0 || j.UpdateAt == 0 || j.RetainUntil == 0 { return mmmodel.NewAppError(where, "model.import_job.is_valid.timestamps.app_error", nil, "id="+j.Id, http.StatusBadRequest) } diff --git a/server/model/import_test.go b/server/model/import_test.go index 2276453..4808edb 100644 --- a/server/model/import_test.go +++ b/server/model/import_test.go @@ -84,6 +84,7 @@ func TestImportJob_IsValid(t *testing.T) { "long space title": func(j *ImportJob) { j.ConfirmedSpaceTitle = strings.Repeat("x", ImportSpaceTitleMaxRunes+1) }, "long source name": func(j *ImportJob) { j.SelectedSourceDisplayName = strings.Repeat("x", ImportDisplayNameMaxRunes+1) }, "long error code": func(j *ImportJob) { j.ErrorCode = strings.Repeat("x", ImportErrorCodeMaxRunes+1) }, + "oversize confirm": func(j *ImportJob) { j.Confirmation = make(ImportConfirmation, ImportConfirmationMaxBytes+1) }, } for name, mutate := range tests { t.Run(name, func(t *testing.T) { @@ -149,6 +150,49 @@ func TestImportIssueRecord_IsValid(t *testing.T) { } } +func TestImportConfirmation_ValueAndScan(t *testing.T) { + // Empty confirmation persists as an empty JSON object (matches the column's NOT NULL DEFAULT). + empty := ImportConfirmation(nil) + v, err := empty.Value() + if err != nil { + t.Fatalf("empty Value: %v", err) + } + if v != "{}" { + t.Errorf("empty Value = %v, want {}", v) + } + + // A payload larger than 1 MiB (which mmmodel.StringInterface's Value() would reject) must be + // accepted, since a valid confirmation with thousands of conflict descriptors exceeds 1 MiB. + oneAndHalfMiB := ImportConfirmation(`{"overwrite_conflicts":[` + strings.Repeat("0", 1_500_000) + `]}`) + if _, err := oneAndHalfMiB.Value(); err != nil { + t.Fatalf("1.5 MiB confirmation should persist, got %v", err) + } + // Confirm StringInterface would have rejected the same size, i.e. our new type is what unblocks it. + big := make(mmmodel.StringInterface) + big["blob"] = strings.Repeat("x", 1_500_000) + if _, siErr := big.Value(); siErr == nil { + t.Errorf("expected StringInterface to reject a >1 MiB payload (sanity check on the motivation)") + } + + // Over the deliberate cap is rejected as a backstop. + over := ImportConfirmation(strings.Repeat("x", ImportConfirmationMaxBytes+1)) + if _, err := over.Value(); err == nil { + t.Errorf("expected rejection above ImportConfirmationMaxBytes") + } + + // Round-trips through Scan for both []byte and string sources. + var c ImportConfirmation + if err := c.Scan([]byte(`{"a":1}`)); err != nil || string(c) != `{"a":1}` { + t.Errorf("Scan([]byte) = %q, %v", string(c), err) + } + if err := c.Scan(`{"b":2}`); err != nil || string(c) != `{"b":2}` { + t.Errorf("Scan(string) = %q, %v", string(c), err) + } + if err := c.Scan(nil); err != nil || c != nil { + t.Errorf("Scan(nil) = %q, %v", string(c), err) + } +} + func TestNewImportFidelity(t *testing.T) { f := NewImportFidelity() if f.FullFidelity {