diff --git a/apps/console/src/pages/organizations/documents/_components/CreateDocumentDialog.tsx b/apps/console/src/pages/organizations/documents/_components/CreateDocumentDialog.tsx index f99d5801d..30448a66b 100644 --- a/apps/console/src/pages/organizations/documents/_components/CreateDocumentDialog.tsx +++ b/apps/console/src/pages/organizations/documents/_components/CreateDocumentDialog.tsx @@ -66,7 +66,6 @@ const createDocumentMutation = graphql` const documentSchema = z.object({ title: z.string().min(1, "Title is required"), - approverIds: z.array(z.string()).min(1, "At least one approver is required"), documentType: z.enum(["OTHER", "GOVERNANCE", "POLICY", "PROCEDURE", "PLAN", "REGISTER", "RECORD", "REPORT", "TEMPLATE"]), classification: z.enum(["PUBLIC", "INTERNAL", "CONFIDENTIAL", "SECRET"]), }); diff --git a/packages/ui/src/RichEditor/LinkExtension.ts b/packages/ui/src/RichEditor/LinkExtension.ts index ddeb16e69..2b393e9d5 100644 --- a/packages/ui/src/RichEditor/LinkExtension.ts +++ b/packages/ui/src/RichEditor/LinkExtension.ts @@ -22,6 +22,9 @@ export const LinkExtension = Link.extend({ view.dom.classList.remove("pointer-on-hovered-link"); } }, + blur: (view) => { + view.dom.classList.remove("pointer-on-hovered-link"); + }, }, handleClick: (_view, _, event) => { const { ctrlKey, metaKey } = event; // Check for Ctrl (Windows) or Cmd (Mac) diff --git a/packages/ui/src/RichEditor/TableColumnMenu/TableColumnMenu.tsx b/packages/ui/src/RichEditor/TableColumnMenu/TableColumnMenu.tsx index 842a28a32..054006450 100644 --- a/packages/ui/src/RichEditor/TableColumnMenu/TableColumnMenu.tsx +++ b/packages/ui/src/RichEditor/TableColumnMenu/TableColumnMenu.tsx @@ -70,6 +70,11 @@ export function moveColumn( const table = editor.state.doc.nodeAt(tableNodePos); if (!table) return; + const map = TableMap.get(table); + if (new Set(map.map).size < map.map.length) return; + if (fromCol < 0 || fromCol >= map.width || toCol < 0 || toCol >= map.width) + return; + const rows: PMNode[] = []; table.forEach((row) => { const cells: PMNode[] = []; diff --git a/packages/ui/src/RichEditor/TableColumnMenu/TableColumnMenuContent.tsx b/packages/ui/src/RichEditor/TableColumnMenu/TableColumnMenuContent.tsx index 4428855e5..ff70ef183 100644 --- a/packages/ui/src/RichEditor/TableColumnMenu/TableColumnMenuContent.tsx +++ b/packages/ui/src/RichEditor/TableColumnMenu/TableColumnMenuContent.tsx @@ -129,8 +129,12 @@ export function TableColumnMenuContent({ .chain() .focus() .command(({ tr }) => { + const seen = new Set(); for (let row = map.height - 1; row >= 0; row--) { const cellOffset = map.map[row * map.width + colIndex]; + if (seen.has(cellOffset)) continue; + seen.add(cellOffset); + const cell = table.nodeAt(cellOffset); if (!cell) continue; @@ -211,13 +215,17 @@ export function TableColumnMenuContent({ const lastCellPos = map.map[(map.height - 1) * map.width + colIndex] + tableStart + 1; - const $anchor = editor.state.doc.resolve(firstCellPos); - const $head = editor.state.doc.resolve(lastCellPos); - - editor.view.dispatch( - editor.state.tr.setSelection(new CellSelection($anchor, $head)), - ); - editor.commands.deleteSelection(); + editor + .chain() + .focus() + .command(({ tr }) => { + const $anchor = tr.doc.resolve(firstCellPos); + const $head = tr.doc.resolve(lastCellPos); + tr.setSelection(new CellSelection($anchor, $head)); + return true; + }) + .deleteSelection() + .run(); } catch { // table may have changed } diff --git a/packages/ui/src/RichEditor/TableRowMenu/TableRowMenu.tsx b/packages/ui/src/RichEditor/TableRowMenu/TableRowMenu.tsx index 87108fbbc..2999d7e13 100644 --- a/packages/ui/src/RichEditor/TableRowMenu/TableRowMenu.tsx +++ b/packages/ui/src/RichEditor/TableRowMenu/TableRowMenu.tsx @@ -70,6 +70,11 @@ export function moveRow( const table = editor.state.doc.nodeAt(tableNodePos); if (!table) return; + const map = TableMap.get(table); + if (new Set(map.map).size < map.map.length) return; + if (fromRow < 0 || fromRow >= map.height || toRow < 0 || toRow >= map.height) + return; + const rows: PMNode[] = []; table.forEach(row => rows.push(row)); const [moved] = rows.splice(fromRow, 1); diff --git a/packages/ui/src/RichEditor/TableRowMenu/TableRowMenuContent.tsx b/packages/ui/src/RichEditor/TableRowMenu/TableRowMenuContent.tsx index 915e22d4a..cf83a4424 100644 --- a/packages/ui/src/RichEditor/TableRowMenu/TableRowMenuContent.tsx +++ b/packages/ui/src/RichEditor/TableRowMenu/TableRowMenuContent.tsx @@ -209,13 +209,17 @@ export function TableRowMenuContent({ const lastCellPos = map.map[rowIndex * map.width + (map.width - 1)] + tableStart; - const $anchor = editor.state.doc.resolve(firstCellPos); - const $head = editor.state.doc.resolve(lastCellPos); - - editor.view.dispatch( - editor.state.tr.setSelection(new CellSelection($anchor, $head)), - ); - editor.commands.deleteSelection(); + editor + .chain() + .focus() + .command(({ tr }) => { + const $anchor = tr.doc.resolve(firstCellPos); + const $head = tr.doc.resolve(lastCellPos); + tr.setSelection(new CellSelection($anchor, $head)); + return true; + }) + .deleteSelection() + .run(); } catch { // table may have changed } diff --git a/pkg/probo/document_service.go b/pkg/probo/document_service.go index 92f0617a4..39472d93c 100644 --- a/pkg/probo/document_service.go +++ b/pkg/probo/document_service.go @@ -153,8 +153,6 @@ func (udvr *UpdateDocumentVersionRequest) Validate() error { v.Check( udvr.Content, "content", - validator.Required(), - validator.NotEmpty(), validator.MaxLen(documentMaxLength), validator.ProseMirrorDocumentContent(), ) @@ -783,17 +781,15 @@ func (s *DocumentService) UpdateVersion( return &ErrDocumentVersionNotDraft{} } - var content string if req.Content != nil { - var err error - content, err = prosemirror.SanitizeDocumentJSON(*req.Content) + content, err := prosemirror.SanitizeDocumentJSON(*req.Content) if err != nil { return fmt.Errorf("cannot sanitize document content: %w", err) } + documentVersion.Content = content } documentVersion.Title = document.Title - documentVersion.Content = content if req.Classification != nil { documentVersion.Classification = *req.Classification } diff --git a/pkg/prosemirror/html.go b/pkg/prosemirror/html.go index f4383fc18..c59b0782e 100644 --- a/pkg/prosemirror/html.go +++ b/pkg/prosemirror/html.go @@ -100,7 +100,7 @@ func renderNode(buf *bytes.Buffer, n Node) error { return fmt.Errorf("cannot render image node: %w", err) } buf.WriteString(" 1 && (src[1] == '/' || src[1] == '\\') { + return "" + } + return src + } + u, err := url.Parse(src) + if err != nil { + return "" + } + if u.Scheme != "" { + switch strings.ToLower(u.Scheme) { + case "http", "https", "data": + return src + default: + return "" + } + } + if u.Host != "" { + return "" + } + return src +} diff --git a/pkg/prosemirror/html_test.go b/pkg/prosemirror/html_test.go index 8333383af..33e98f625 100644 --- a/pkg/prosemirror/html_test.go +++ b/pkg/prosemirror/html_test.go @@ -222,6 +222,26 @@ func TestRenderHTML_LinkBlankTargetDefaultRel(t *testing.T) { raw: `{"type":"text","marks":[{"type":"link","attrs":{"href":"https://example.com","target":" _blank ","rel":" "}}],"text":"hi"}`, want: `hi`, }, + { + name: "custom rel without noopener gets noopener appended", + raw: `{"type":"text","marks":[{"type":"link","attrs":{"href":"https://example.com","target":"_blank","rel":"nofollow"}}],"text":"hi"}`, + want: `hi`, + }, + { + name: "custom rel already has noopener", + raw: `{"type":"text","marks":[{"type":"link","attrs":{"href":"https://example.com","target":"_blank","rel":"noopener nofollow"}}],"text":"hi"}`, + want: `hi`, + }, + { + name: "custom rel with noopener case insensitive", + raw: `{"type":"text","marks":[{"type":"link","attrs":{"href":"https://example.com","target":"_blank","rel":"NoOpener"}}],"text":"hi"}`, + want: `hi`, + }, + { + name: "custom rel without blank target unchanged", + raw: `{"type":"text","marks":[{"type":"link","attrs":{"href":"https://example.com","target":"_self","rel":"nofollow"}}],"text":"hi"}`, + want: `hi`, + }, } { t.Run( tc.name, @@ -293,6 +313,47 @@ func TestRenderHTML_Image(t *testing.T) { assert.Equal(t, `A photo`, got) } +func TestRenderHTML_ImageSanitizesDangerousSrc(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + src string + wantSrc string + }{ + {name: "javascript scheme", src: `javascript:alert(1)`, wantSrc: ``}, + {name: "javascript case insensitive", src: `javaScript:alert(1)`, wantSrc: ``}, + {name: "vbscript scheme", src: `vbscript:MsgBox("xss")`, wantSrc: ``}, + {name: "protocol-relative", src: `//evil.example/img.png`, wantSrc: ``}, + {name: "empty src", src: ``, wantSrc: ``}, + {name: "https preserved", src: `https://example.com/img.png`, wantSrc: `https://example.com/img.png`}, + {name: "http preserved", src: `http://example.com/img.png`, wantSrc: `http://example.com/img.png`}, + {name: "data URI preserved", src: `data:image/png;base64,iVBOR`, wantSrc: `data:image/png;base64,iVBOR`}, + {name: "absolute path", src: `/images/photo.png`, wantSrc: `/images/photo.png`}, + {name: "relative path", src: `images/photo.png`, wantSrc: `images/photo.png`}, + } { + t.Run( + tc.name, + func(t *testing.T) { + t.Parallel() + srcJSON, err := json.Marshal(tc.src) + require.NoError(t, err) + raw := fmt.Sprintf( + `{"type":"image","attrs":{"src":%s}}`, + string(srcJSON), + ) + var n Node + require.NoError(t, json.Unmarshal([]byte(raw), &n)) + + got, err := RenderHTML(n) + require.NoError(t, err) + want := fmt.Sprintf(``, html.EscapeString(tc.wantSrc)) + assert.Equal(t, want, got) + }, + ) + } +} + func TestRenderHTML_MultipleMarks(t *testing.T) { t.Parallel() diff --git a/pkg/prosemirror/markdown.go b/pkg/prosemirror/markdown.go index 468e71eae..e13ce1511 100644 --- a/pkg/prosemirror/markdown.go +++ b/pkg/prosemirror/markdown.go @@ -330,15 +330,7 @@ func (c *converter) convertImage(n *ast.Image) ([]Node, error) { imgAttrs.Title = &t } - // Collect alt text from child text nodes. - var altBuf bytes.Buffer - for child := n.FirstChild(); child != nil; child = child.NextSibling() { - if child.Kind() == ast.KindText { - altBuf.Write(child.(*ast.Text).Segment.Value(c.source)) - } - } - if altBuf.Len() > 0 { - alt := altBuf.String() + if alt := c.extractText(n); alt != "" { imgAttrs.Alt = &alt } @@ -524,6 +516,22 @@ func (c *converter) convertStrikethrough(n ast.Node) ([]Node, error) { return children, nil } +// extractText recursively collects the text content of all descendant nodes. +func (c *converter) extractText(n ast.Node) string { + var buf bytes.Buffer + for child := n.FirstChild(); child != nil; child = child.NextSibling() { + switch child.Kind() { + case ast.KindText: + buf.Write(child.(*ast.Text).Segment.Value(c.source)) + case ast.KindString: + buf.Write(child.(*ast.String).Value) + default: + buf.WriteString(c.extractText(child)) + } + } + return buf.String() +} + func copyMarks(marks []Mark) []Mark { if len(marks) == 0 { return nil diff --git a/pkg/prosemirror/markdown_test.go b/pkg/prosemirror/markdown_test.go index f786bf3a6..53030b78e 100644 --- a/pkg/prosemirror/markdown_test.go +++ b/pkg/prosemirror/markdown_test.go @@ -255,6 +255,25 @@ func TestParseMarkdown_Image(t *testing.T) { assert.Equal(t, "title", *attrs.Title) } +func TestParseMarkdown_ImageFormattedAltText(t *testing.T) { + t.Parallel() + + doc, err := ParseMarkdown("![**bold** and *italic*](https://example.com/img.png)") + require.NoError(t, err) + require.Len(t, doc.Content, 1) + + p := doc.Content[0] + require.Len(t, p.Content, 1) + + img := p.Content[0] + assert.Equal(t, NodeImage, img.Type) + + attrs, err := img.ImageAttrs() + require.NoError(t, err) + require.NotNil(t, attrs.Alt) + assert.Equal(t, "bold and italic", *attrs.Alt) +} + func TestParseMarkdown_BulletList(t *testing.T) { t.Parallel() diff --git a/pkg/prosemirror/sanitize.go b/pkg/prosemirror/sanitize.go index 922c0dc5c..163afe48c 100644 --- a/pkg/prosemirror/sanitize.go +++ b/pkg/prosemirror/sanitize.go @@ -66,6 +66,9 @@ func SanitizeDocumentJSON(s string) (string, error) { } func sanitizeNode(n *Node) { + if n.Type == NodeImage { + sanitizeImageNode(n) + } for i := range n.Marks { sanitizeLinkMark(&n.Marks[i]) } @@ -74,6 +77,23 @@ func sanitizeNode(n *Node) { } } +func sanitizeImageNode(n *Node) { + attrs, err := n.ImageAttrs() + if err != nil { + n.Attrs = []byte(`{"src":""}`) + return + } + + attrs.Src = safeImageSrc(attrs.Src) + raw, err := json.Marshal(attrs) + if err != nil { + n.Attrs = []byte(`{"src":""}`) + return + } + + n.Attrs = raw +} + func sanitizeLinkMark(m *Mark) { if m.Type != MarkLink { return diff --git a/pkg/prosemirror/sanitize_test.go b/pkg/prosemirror/sanitize_test.go index 696fc58e4..26aa234ba 100644 --- a/pkg/prosemirror/sanitize_test.go +++ b/pkg/prosemirror/sanitize_test.go @@ -82,3 +82,37 @@ func TestSanitizeDocumentJSON_PreservesSafeHref(t *testing.T) { require.NoError(t, err) assert.Equal(t, "https://example.com", attrs.Href) } + +func TestSanitizeDocumentJSON_ImageSrc(t *testing.T) { + t.Parallel() + + raw := `{"type":"doc","content":[{"type":"image","attrs":{"src":"javascript:alert(1)","alt":"xss"}}]}` + + out, err := SanitizeDocumentJSON(raw) + require.NoError(t, err) + + var doc Node + require.NoError(t, json.Unmarshal([]byte(out), &doc)) + img := doc.Content[0] + attrs, err := img.ImageAttrs() + require.NoError(t, err) + assert.Equal(t, "", attrs.Src) + require.NotNil(t, attrs.Alt) + assert.Equal(t, "xss", *attrs.Alt) +} + +func TestSanitizeDocumentJSON_PreservesSafeImageSrc(t *testing.T) { + t.Parallel() + + raw := `{"type":"doc","content":[{"type":"image","attrs":{"src":"https://example.com/img.png","alt":"ok"}}]}` + + out, err := SanitizeDocumentJSON(raw) + require.NoError(t, err) + + var doc Node + require.NoError(t, json.Unmarshal([]byte(out), &doc)) + img := doc.Content[0] + attrs, err := img.ImageAttrs() + require.NoError(t, err) + assert.Equal(t, "https://example.com/img.png", attrs.Src) +} diff --git a/pkg/validator/validator_prosemirror.go b/pkg/validator/validator_prosemirror.go index 151cb8170..776909d39 100644 --- a/pkg/validator/validator_prosemirror.go +++ b/pkg/validator/validator_prosemirror.go @@ -25,7 +25,11 @@ import ( // strings are allowed. func ProseMirrorDocumentContent() ValidatorFunc { return func(value any) *ValidationError { - s, ok := value.(string) + actualValue, isNil := dereferenceValue(value) + if isNil { + return nil + } + s, ok := actualValue.(string) if !ok { return newValidationError(ErrorCodeInvalidFormat, "value must be a string") } diff --git a/pkg/validator/validator_prosemirror_test.go b/pkg/validator/validator_prosemirror_test.go index 6e94e7a7b..f072f3ec0 100644 --- a/pkg/validator/validator_prosemirror_test.go +++ b/pkg/validator/validator_prosemirror_test.go @@ -31,6 +31,8 @@ func TestProseMirrorDocumentContent(t *testing.T) { {"valid doc", validDoc, false}, {"plain text", "not json", true}, {"non-doc root", `{"type":"paragraph","content":[]}`, true}, + {"nil value", nil, false}, + {"nil *string", (*string)(nil), false}, {"non-string", 1, true}, }