From f3982f23f99aabe95d08790d0ecff45040b40ef8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89mile=20R=C3=A9?= Date: Mon, 30 Mar 2026 17:52:25 +0400 Subject: [PATCH] Sanitize upon saving MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Émile Ré --- .../description/DocumentDescriptionPage.tsx | 10 +- e2e/console/document_test.go | 38 ++--- e2e/console/document_version_test.go | 8 +- e2e/console/mapping_test.go | 4 +- .../internal/testutil/prosemirror.go | 32 ++-- .../updatecontent/updatecontent.go | 140 ------------------ pkg/cmd/root/root.go | 2 - pkg/probo/document_service.go | 78 +++++----- pkg/prosemirror/html.go | 40 ++++- pkg/prosemirror/html_block.go | 2 +- pkg/prosemirror/html_test.go | 45 ++++++ pkg/prosemirror/markdown.go | 4 +- pkg/prosemirror/markdown_test.go | 27 ++++ pkg/prosemirror/sanitize.go | 96 ++++++++++++ pkg/prosemirror/sanitize_test.go | 84 +++++++++++ pkg/server/api/connect/v1/v1_resolver.go | 14 -- pkg/server/api/console/v1/schema.graphql | 10 -- pkg/server/api/console/v1/v1_resolver.go | 32 ---- pkg/server/api/mcp/v1/schema.resolvers.go | 26 ---- pkg/server/api/mcp/v1/specification.yaml | 30 ---- pkg/server/api/trust/v1/v1_resolver.go | 14 -- pkg/validator/validator_prosemirror.go | 40 +++++ pkg/validator/validator_prosemirror_test.go | 51 +++++++ 23 files changed, 471 insertions(+), 356 deletions(-) rename pkg/cmd/documentversion/documentversion.go => e2e/internal/testutil/prosemirror.go (61%) delete mode 100644 pkg/cmd/documentversion/updatecontent/updatecontent.go create mode 100644 pkg/prosemirror/sanitize.go create mode 100644 pkg/prosemirror/sanitize_test.go create mode 100644 pkg/validator/validator_prosemirror.go create mode 100644 pkg/validator/validator_prosemirror_test.go diff --git a/apps/console/src/pages/organizations/documents/description/DocumentDescriptionPage.tsx b/apps/console/src/pages/organizations/documents/description/DocumentDescriptionPage.tsx index 54c4380c5..6202aa8d0 100644 --- a/apps/console/src/pages/organizations/documents/description/DocumentDescriptionPage.tsx +++ b/apps/console/src/pages/organizations/documents/description/DocumentDescriptionPage.tsx @@ -55,9 +55,11 @@ export const documentDescriptionPageQuery = graphql` `; const updateContentMutation = graphql` - mutation DocumentDescriptionPage_updateContentMutation($input: UpdateDocumentVersionContentInput!) { - updateDocumentVersionContent(input: $input) { - content + mutation DocumentDescriptionPage_updateContentMutation($input: UpdateDocumentVersionInput!) { + updateDocumentVersion(input: $input) { + documentVersion { + content + } } } `; @@ -86,7 +88,7 @@ export function DocumentDescriptionPage(props: { queryRef: PreloadedQueryalert('xss')", - "content": "Test content", + "content": testutil.ProseMirrorTextDoc("Test content"), "documentType": "POLICY", "classification": "INTERNAL", }, @@ -178,7 +178,7 @@ func TestDocument_Create_Validation(t *testing.T) { name: "title with newline", input: map[string]any{ "title": "Test\nDocument", - "content": "Test content", + "content": testutil.ProseMirrorTextDoc("Test content"), "documentType": "POLICY", "classification": "INTERNAL", }, @@ -188,7 +188,7 @@ func TestDocument_Create_Validation(t *testing.T) { name: "title with carriage return", input: map[string]any{ "title": "Test\rDocument", - "content": "Test content", + "content": testutil.ProseMirrorTextDoc("Test content"), "documentType": "POLICY", "classification": "INTERNAL", }, @@ -198,7 +198,7 @@ func TestDocument_Create_Validation(t *testing.T) { name: "title with null byte", input: map[string]any{ "title": "Test\x00Document", - "content": "Test content", + "content": testutil.ProseMirrorTextDoc("Test content"), "documentType": "POLICY", "classification": "INTERNAL", }, @@ -208,7 +208,7 @@ func TestDocument_Create_Validation(t *testing.T) { name: "title with tab character", input: map[string]any{ "title": "Test\tDocument", - "content": "Test content", + "content": testutil.ProseMirrorTextDoc("Test content"), "documentType": "POLICY", "classification": "INTERNAL", }, @@ -218,7 +218,7 @@ func TestDocument_Create_Validation(t *testing.T) { name: "title with zero-width space", input: map[string]any{ "title": "Test\u200BDocument", - "content": "Test content", + "content": testutil.ProseMirrorTextDoc("Test content"), "documentType": "POLICY", "classification": "INTERNAL", }, @@ -228,7 +228,7 @@ func TestDocument_Create_Validation(t *testing.T) { name: "title with zero-width joiner", input: map[string]any{ "title": "Test\u200DDocument", - "content": "Test content", + "content": testutil.ProseMirrorTextDoc("Test content"), "documentType": "POLICY", "classification": "INTERNAL", }, @@ -238,7 +238,7 @@ func TestDocument_Create_Validation(t *testing.T) { name: "title with right-to-left override", input: map[string]any{ "title": "Test\u202EDocument", - "content": "Test content", + "content": testutil.ProseMirrorTextDoc("Test content"), "documentType": "POLICY", "classification": "INTERNAL", }, @@ -611,7 +611,7 @@ func TestDocument_Timestamps(t *testing.T) { "input": map[string]any{ "organizationId": owner.GetOrganizationID().String(), "title": "Timestamp Test Document", - "content": "Test content", + "content": testutil.ProseMirrorTextDoc("Test content"), "documentType": "POLICY", "classification": "INTERNAL", }, @@ -740,7 +740,7 @@ func TestDocument_RBAC(t *testing.T) { "input": map[string]any{ "organizationId": owner.GetOrganizationID().String(), "title": "RBAC Test Document", - "content": "Test content", + "content": testutil.ProseMirrorTextDoc("Test content"), "documentType": "POLICY", "classification": "INTERNAL", }, @@ -762,7 +762,7 @@ func TestDocument_RBAC(t *testing.T) { "input": map[string]any{ "organizationId": admin.GetOrganizationID().String(), "title": "RBAC Test Document", - "content": "Test content", + "content": testutil.ProseMirrorTextDoc("Test content"), "documentType": "POLICY", "classification": "INTERNAL", }, @@ -784,7 +784,7 @@ func TestDocument_RBAC(t *testing.T) { "input": map[string]any{ "organizationId": viewer.GetOrganizationID().String(), "title": "RBAC Test Document", - "content": "Test content", + "content": testutil.ProseMirrorTextDoc("Test content"), "documentType": "POLICY", "classification": "INTERNAL", }, @@ -1007,7 +1007,7 @@ func TestDocument_MaxLength_Validation(t *testing.T) { "input": map[string]any{ "organizationId": owner.GetOrganizationID().String(), "title": longTitle, - "content": "Test content", + "content": testutil.ProseMirrorTextDoc("Test content"), "documentType": "POLICY", "classification": "INTERNAL", }, diff --git a/e2e/console/document_version_test.go b/e2e/console/document_version_test.go index 0f83e6b02..f8cbdfce9 100644 --- a/e2e/console/document_version_test.go +++ b/e2e/console/document_version_test.go @@ -73,7 +73,7 @@ func createTestDocument(t *testing.T, owner *testutil.Client) (docID string, doc "input": map[string]any{ "organizationId": owner.GetOrganizationID().String(), "title": "Test Document", - "content": "Initial content", + "content": testutil.ProseMirrorTextDoc("Initial content"), "documentType": "POLICY", "classification": "INTERNAL", }, @@ -282,15 +282,17 @@ func TestDocumentVersion_UpdateContent(t *testing.T) { } `json:"updateDocumentVersionContent"` } + wantContent := testutil.ProseMirrorTextDoc("Updated content for the document version") + err := owner.Execute(query, map[string]any{ "input": map[string]any{ "id": draftVersionID, - "content": "Updated content for the document version", + "content": wantContent, }, }, &result) require.NoError(t, err) - assert.Equal(t, "Updated content for the document version", result.UpdateDocumentVersionContent.Content) + assert.JSONEq(t, wantContent, result.UpdateDocumentVersionContent.Content) } func TestDocumentVersion_RequestSignature(t *testing.T) { diff --git a/e2e/console/mapping_test.go b/e2e/console/mapping_test.go index a6c40f228..b4d1369b4 100644 --- a/e2e/console/mapping_test.go +++ b/e2e/console/mapping_test.go @@ -393,7 +393,7 @@ func TestControlDocumentMapping_CreateDelete(t *testing.T) { "input": map[string]any{ "organizationId": owner.GetOrganizationID().String(), "title": "Document for Control Mapping", - "content": "Document content", + "content": testutil.ProseMirrorTextDoc("Document content"), "documentType": "POLICY", "classification": "INTERNAL", }, @@ -781,7 +781,7 @@ func TestRiskDocumentMapping_CreateDelete(t *testing.T) { "input": map[string]any{ "organizationId": owner.GetOrganizationID().String(), "title": "Document for Risk Mapping", - "content": "Document content", + "content": testutil.ProseMirrorTextDoc("Document content"), "documentType": "POLICY", "classification": "INTERNAL", }, diff --git a/pkg/cmd/documentversion/documentversion.go b/e2e/internal/testutil/prosemirror.go similarity index 61% rename from pkg/cmd/documentversion/documentversion.go rename to e2e/internal/testutil/prosemirror.go index 8a19a8f53..90efe8c36 100644 --- a/pkg/cmd/documentversion/documentversion.go +++ b/e2e/internal/testutil/prosemirror.go @@ -12,21 +12,29 @@ // OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. -package documentversion +package testutil import ( - "github.com/spf13/cobra" - "go.probo.inc/probo/pkg/cmd/cmdutil" - "go.probo.inc/probo/pkg/cmd/documentversion/updatecontent" + "encoding/json" ) -func NewCmdDocumentVersion(f *cmdutil.Factory) *cobra.Command { - cmd := &cobra.Command{ - Use: "document-version ", - Short: "Manage document versions", +// ProseMirrorTextDoc returns minimal ProseMirror/Tiptap JSON accepted by the API: +// a doc with one paragraph containing a single text node. +func ProseMirrorTextDoc(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}, + }, + }, + }, } - - cmd.AddCommand(updatecontent.NewCmdUpdateContent(f)) - - return cmd + b, err := json.Marshal(doc) + if err != nil { + panic(err) + } + return string(b) } diff --git a/pkg/cmd/documentversion/updatecontent/updatecontent.go b/pkg/cmd/documentversion/updatecontent/updatecontent.go deleted file mode 100644 index 243384c38..000000000 --- a/pkg/cmd/documentversion/updatecontent/updatecontent.go +++ /dev/null @@ -1,140 +0,0 @@ -// Copyright (c) 2026 Probo Inc . -// -// Permission to use, copy, modify, and/or distribute this software for any -// purpose with or without fee is hereby granted, provided that the above -// copyright notice and this permission notice appear in all copies. -// -// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH -// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY -// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, -// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM -// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR -// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR -// PERFORMANCE OF THIS SOFTWARE. - -package updatecontent - -import ( - "encoding/json" - "fmt" - "io" - - "github.com/spf13/cobra" - "go.probo.inc/probo/pkg/cli/api" - "go.probo.inc/probo/pkg/cmd/cmdutil" - "go.probo.inc/probo/pkg/prosemirror" -) - -const updateContentMutation = ` -mutation($input: UpdateDocumentVersionContentInput!) { - updateDocumentVersionContent(input: $input) { - content - } -} -` - -type updateContentResponse struct { - UpdateDocumentVersionContent struct { - Content string `json:"content"` - } `json:"updateDocumentVersionContent"` -} - -func NewCmdUpdateContent(f *cmdutil.Factory) *cobra.Command { - var ( - flagID string - flagContent string - flagFromMarkdown string - ) - - cmd := &cobra.Command{ - Use: "update-content", - Short: "Update document version content", - Example: ` # Update with ProseMirror JSON - prb document-version update-content --id --content '{"type":"doc",...}' - - # Update from markdown - prb document-version update-content --id --from-markdown "# Hello" - - # Update from stdin - cat content.json | prb document-version update-content --id `, - RunE: func(cmd *cobra.Command, args []string) error { - cfg, err := f.Config() - if err != nil { - return err - } - - host, hc, err := cfg.DefaultHost() - if err != nil { - return err - } - - client := api.NewClient( - host, - hc.Token, - "/api/console/v1/graphql", - cfg.HTTPTimeoutDuration(), - ) - - var content string - switch { - case flagFromMarkdown != "": - doc, err := prosemirror.ParseMarkdown(flagFromMarkdown) - if err != nil { - return err - } - out, err := json.Marshal(doc) - if err != nil { - return fmt.Errorf("cannot marshal prosemirror document: %w", err) - } - content = string(out) - case flagContent != "": - content = flagContent - default: - data, err := io.ReadAll(f.IOStreams.In) - if err != nil { - return fmt.Errorf("cannot read from stdin: %w", err) - } - content = string(data) - } - - input := map[string]any{ - "id": flagID, - "content": content, - } - - data, err := client.Do( - updateContentMutation, - map[string]any{"input": input}, - ) - if err != nil { - return err - } - - var resp updateContentResponse - if err := json.Unmarshal(data, &resp); err != nil { - return fmt.Errorf("cannot parse response: %w", err) - } - - _, _ = fmt.Fprintf( - f.IOStreams.Out, - "Updated document version content %s\n", - flagID, - ) - - return nil - }, - } - - cmd.Flags().StringVar(&flagID, "id", "", "Document version ID (required)") - cmd.Flags().StringVar(&flagContent, "content", "", "ProseMirror JSON content") - cmd.Flags().StringVar( - &flagFromMarkdown, - "from-markdown", - "", - "Markdown content to convert and upload", - ) - - _ = cmd.MarkFlagRequired("id") - - return cmd -} diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index c0f33aaac..55fe74ba6 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -25,7 +25,6 @@ import ( cmdconfig "go.probo.inc/probo/pkg/cmd/config" cmdcontext "go.probo.inc/probo/pkg/cmd/context" "go.probo.inc/probo/pkg/cmd/control" - "go.probo.inc/probo/pkg/cmd/documentversion" "go.probo.inc/probo/pkg/cmd/evidence" "go.probo.inc/probo/pkg/cmd/finding" "go.probo.inc/probo/pkg/cmd/framework" @@ -75,7 +74,6 @@ func NewCmdRoot(f *cmdutil.Factory) *cobra.Command { cmd.AddCommand(cmdconfig.NewCmdConfig(f)) cmd.AddCommand(cmdcontext.NewCmdContext(f)) cmd.AddCommand(control.NewCmdControl(f)) - cmd.AddCommand(documentversion.NewCmdDocumentVersion(f)) cmd.AddCommand(evidence.NewCmdEvidence(f)) cmd.AddCommand(finding.NewCmdFinding(f)) cmd.AddCommand(framework.NewCmdFramework(f)) diff --git a/pkg/probo/document_service.go b/pkg/probo/document_service.go index 41f92d3c5..92f0617a4 100644 --- a/pkg/probo/document_service.go +++ b/pkg/probo/document_service.go @@ -40,6 +40,7 @@ import ( "go.probo.inc/probo/pkg/iam" "go.probo.inc/probo/pkg/mail" "go.probo.inc/probo/pkg/page" + "go.probo.inc/probo/pkg/prosemirror" "go.probo.inc/probo/pkg/statelesstoken" "go.probo.inc/probo/pkg/validator" "go.probo.inc/probo/pkg/watermarkpdf" @@ -120,7 +121,12 @@ func (cdr *CreateDocumentRequest) Validate() error { v.Check(cdr.OrganizationID, "organization_id", validator.Required(), validator.GID(coredata.OrganizationEntityType)) v.Check(cdr.Title, "title", validator.Required(), validator.SafeTextNoNewLine(TitleMaxLength)) - v.Check(cdr.Content, "content", validator.MaxLen(documentMaxLength)) + v.Check( + cdr.Content, + "content", + validator.MaxLen(documentMaxLength), + validator.ProseMirrorDocumentContent(), + ) v.Check(cdr.Classification, "classification", validator.Required(), validator.OneOfSlice(coredata.DocumentClassifications())) v.Check(cdr.DocumentType, "document_type", validator.Required(), validator.OneOfSlice(coredata.DocumentTypes())) v.Check(cdr.TrustCenterVisibility, "trust_center_visibility", validator.OneOfSlice(coredata.TrustCenterVisibilities())) @@ -143,8 +149,15 @@ func (udvr *UpdateDocumentVersionRequest) Validate() error { v := validator.New() v.Check(udvr.ID, "id", validator.Required(), validator.GID(coredata.DocumentVersionEntityType)) - v.Check(udvr.Content, "content", validator.NotEmpty(), validator.MaxLen(documentMaxLength)) v.Check(udvr.Classification, "classification", validator.OneOfSlice(coredata.DocumentClassifications())) + v.Check( + udvr.Content, + "content", + validator.Required(), + validator.NotEmpty(), + validator.MaxLen(documentMaxLength), + validator.ProseMirrorDocumentContent(), + ) return v.Error() } @@ -518,13 +531,22 @@ func (s *DocumentService) Create( document.TrustCenterVisibility = *req.TrustCenterVisibility } + content := req.Content + if strings.TrimSpace(content) != "" { + var sanitizeErr error + content, sanitizeErr = prosemirror.SanitizeDocumentJSON(content) + if sanitizeErr != nil { + return nil, nil, fmt.Errorf("cannot sanitize document content: %w", sanitizeErr) + } + } + documentVersion := &coredata.DocumentVersion{ ID: documentVersionID, DocumentID: documentID, Title: req.Title, Major: 0, Minor: 1, - Content: req.Content, + Content: content, Status: coredata.DocumentVersionStatusDraft, Classification: req.Classification, CreatedAt: now, @@ -761,10 +783,17 @@ func (s *DocumentService) UpdateVersion( return &ErrDocumentVersionNotDraft{} } - documentVersion.Title = document.Title + var content string if req.Content != nil { - documentVersion.Content = *req.Content + var err error + content, err = prosemirror.SanitizeDocumentJSON(*req.Content) + if err != nil { + return fmt.Errorf("cannot sanitize document content: %w", err) + } } + + documentVersion.Title = document.Title + documentVersion.Content = content if req.Classification != nil { documentVersion.Classification = *req.Classification } @@ -1576,45 +1605,6 @@ func (s *DocumentService) Update( return document, nil } -func (s *DocumentService) UpdateDocumentVersionContent( - ctx context.Context, - req UpdateDocumentVersionRequest, -) (string, error) { - documentVersion := &coredata.DocumentVersion{} - - if err := req.Validate(); err != nil { - return "", err - } - - err := s.svc.pg.WithTx( - ctx, - func(conn pg.Conn) error { - if err := documentVersion.LoadByID(ctx, conn, s.svc.scope, req.ID); err != nil { - return fmt.Errorf("cannot load document version %q: %w", req.ID, err) - } - - if documentVersion.Status != coredata.DocumentVersionStatusDraft { - return &ErrDocumentVersionNotDraft{} - } - - documentVersion.Content = req.Content - documentVersion.UpdatedAt = time.Now() - - if err := documentVersion.Update(ctx, conn, s.svc.scope); err != nil { - return fmt.Errorf("cannot update document version: %w", err) - } - - return nil - }, - ) - - if err != nil { - return "", err - } - - return documentVersion.Content, nil -} - func (s *DocumentService) Archive( ctx context.Context, documentID gid.GID, diff --git a/pkg/prosemirror/html.go b/pkg/prosemirror/html.go index 39ea95bbb..3d5d57ab3 100644 --- a/pkg/prosemirror/html.go +++ b/pkg/prosemirror/html.go @@ -18,7 +18,9 @@ import ( "bytes" "fmt" "html" + "net/url" "strconv" + "strings" ) // RenderHTML renders a ProseMirror document node tree to an HTML string. @@ -192,7 +194,7 @@ func openMark(buf *bytes.Buffer, m Mark) error { return fmt.Errorf("cannot render link mark: %w", err) } buf.WriteString(" 1 && (href[1] == '/' || href[1] == '\\') { + return "#" + } + return href + } + u, err := url.Parse(href) + if err != nil { + return "#" + } + if u.Scheme != "" { + switch strings.ToLower(u.Scheme) { + case "http", "https", "mailto", "tel": + return href + default: + return "#" + } + } + if u.Host != "" { + return "#" + } + return href +} diff --git a/pkg/prosemirror/html_block.go b/pkg/prosemirror/html_block.go index 636631e0e..b7be1bb7c 100644 --- a/pkg/prosemirror/html_block.go +++ b/pkg/prosemirror/html_block.go @@ -561,7 +561,7 @@ func (c *htmlBlockConverter) convertAnchor(n *html.Node) ([]Node, error) { if t := attrVal(n, "title"); t != "" { title = &t } - attrs, err := json.Marshal(LinkAttrs{Href: href, Title: title}) + attrs, err := json.Marshal(LinkAttrs{Href: safeLinkHref(href), Title: title}) if err != nil { return nil, fmt.Errorf("cannot marshal link attrs: %w", err) } diff --git a/pkg/prosemirror/html_test.go b/pkg/prosemirror/html_test.go index 3c4b21286..b6cfb07bb 100644 --- a/pkg/prosemirror/html_test.go +++ b/pkg/prosemirror/html_test.go @@ -16,6 +16,8 @@ package prosemirror import ( "encoding/json" + "fmt" + "html" "os" "testing" @@ -185,6 +187,49 @@ func TestRenderHTML_LinkMinimalAttrs(t *testing.T) { assert.Equal(t, `hi`, got) } +func TestRenderHTML_LinkSanitizesDangerousHrefs(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + href string + wantHref string + }{ + {name: "javascript scheme", href: `javascript:alert(1)`, wantHref: `#`}, + {name: "javascript scheme case insensitive", href: `javaScript:alert(1)`, wantHref: `#`}, + {name: "data html", href: `data:text/html,`, wantHref: `#`}, + {name: "protocol-relative", href: `//evil.example/phish`, wantHref: `#`}, + {name: "path with leading slash-slash", href: `//not-a-path`, wantHref: `#`}, + {name: "empty href", href: ``, wantHref: `#`}, + {name: "fragment only", href: `#section`, wantHref: `#section`}, + {name: "relative path", href: `docs/page`, wantHref: `docs/page`}, + {name: "absolute path", href: `/app/foo`, wantHref: `/app/foo`}, + {name: "mailto", href: `mailto:user@example.com`, wantHref: `mailto:user@example.com`}, + {name: "tel", href: `tel:+15551212`, wantHref: `tel:+15551212`}, + {name: "https preserved", href: `https://example.com/x`, wantHref: `https://example.com/x`}, + } { + t.Run( + tc.name, + func(t *testing.T) { + t.Parallel() + hrefJSON, err := json.Marshal(tc.href) + require.NoError(t, err) + raw := fmt.Sprintf( + `{"type":"text","marks":[{"type":"link","attrs":{"href":%s,"target":null,"rel":null,"class":null,"title":null}}],"text":"x"}`, + string(hrefJSON), + ) + var n Node + require.NoError(t, json.Unmarshal([]byte(raw), &n)) + + got, err := RenderHTML(n) + require.NoError(t, err) + want := fmt.Sprintf(`x`, html.EscapeString(tc.wantHref)) + assert.Equal(t, want, got) + }, + ) + } +} + func TestRenderHTML_Image(t *testing.T) { t.Parallel() diff --git a/pkg/prosemirror/markdown.go b/pkg/prosemirror/markdown.go index e22bf2427..b7af49d85 100644 --- a/pkg/prosemirror/markdown.go +++ b/pkg/prosemirror/markdown.go @@ -426,7 +426,7 @@ func (c *converter) convertCodeSpan(n ast.Node) ([]Node, error) { func (c *converter) convertLink(n *ast.Link) ([]Node, error) { linkAttrs := LinkAttrs{ - Href: string(n.Destination), + Href: safeLinkHref(string(n.Destination)), } if n.Title != nil { @@ -452,7 +452,7 @@ func (c *converter) convertLink(n *ast.Link) ([]Node, error) { func (c *converter) convertAutoLink(n *ast.AutoLink) ([]Node, error) { url := string(n.URL(c.source)) - linkAttrs := LinkAttrs{Href: url} + linkAttrs := LinkAttrs{Href: safeLinkHref(url)} attrs, err := json.Marshal(linkAttrs) if err != nil { return nil, fmt.Errorf("cannot marshal link attrs: %w", err) diff --git a/pkg/prosemirror/markdown_test.go b/pkg/prosemirror/markdown_test.go index 7cc91cb34..d6136e270 100644 --- a/pkg/prosemirror/markdown_test.go +++ b/pkg/prosemirror/markdown_test.go @@ -205,6 +205,33 @@ func TestParseMarkdown_Link(t *testing.T) { assert.Equal(t, "https://example.com", linkAttrs.Href) } +func TestParseMarkdown_LinkSanitizesDangerousHrefs(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + markdown string + wantHref string + }{ + {name: "javascript scheme", markdown: `[x](javascript:alert(1))`, wantHref: `#`}, + {name: "data html", markdown: `[x](data:text/html,)`, wantHref: `#`}, + {name: "protocol-relative", markdown: `[x](//evil.example/phish)`, wantHref: `#`}, + {name: "https preserved", markdown: `[x](https://example.com/y)`, wantHref: `https://example.com/y`}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + doc, err := ParseMarkdown(tt.markdown) + require.NoError(t, err) + txt := doc.Content[0].Content[0] + linkAttrs, err := txt.Marks[0].LinkAttrs() + require.NoError(t, err) + assert.Equal(t, tt.wantHref, linkAttrs.Href) + }) + } +} + func TestParseMarkdown_Image(t *testing.T) { t.Parallel() diff --git a/pkg/prosemirror/sanitize.go b/pkg/prosemirror/sanitize.go new file mode 100644 index 000000000..922c0dc5c --- /dev/null +++ b/pkg/prosemirror/sanitize.go @@ -0,0 +1,96 @@ +// Copyright (c) 2026 Probo Inc . +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR +// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +package prosemirror + +import ( + "encoding/json" + "fmt" + "strings" +) + +// ValidateDocumentContentJSON returns nil if s is empty or whitespace-only. +// Otherwise s must be valid ProseMirror JSON whose root node has type "doc". +func ValidateDocumentContentJSON(s string) error { + if strings.TrimSpace(s) == "" { + return nil + } + _, err := parseDocRoot(s) + return err +} + +func parseDocRoot(s string) (Node, error) { + n, err := Parse(s) + if err != nil { + return Node{}, fmt.Errorf("cannot parse document content as ProseMirror JSON: %w", err) + } + if n.Type != NodeDoc { + return Node{}, fmt.Errorf("document content root must be type %q", NodeDoc) + } + return n, nil +} + +// SanitizeDocumentJSON parses a ProseMirror/Tiptap JSON document, replaces +// unsafe link mark href values using the same rules as RenderHTML, and +// re-serializes the document. Whitespace-only input is returned unchanged. +// Non-empty content must be valid JSON whose root node has type "doc". +func SanitizeDocumentJSON(s string) (string, error) { + if strings.TrimSpace(s) == "" { + return s, nil + } + + n, err := parseDocRoot(s) + if err != nil { + return "", err + } + + sanitizeNode(&n) + + out, err := json.Marshal(n) + if err != nil { + return "", fmt.Errorf("cannot marshal sanitized document: %w", err) + } + + return string(out), nil +} + +func sanitizeNode(n *Node) { + for i := range n.Marks { + sanitizeLinkMark(&n.Marks[i]) + } + for i := range n.Content { + sanitizeNode(&n.Content[i]) + } +} + +func sanitizeLinkMark(m *Mark) { + if m.Type != MarkLink { + return + } + + attrs, err := m.LinkAttrs() + if err != nil { + m.Attrs = []byte(`{"href":"#"}`) + return + } + + attrs.Href = safeLinkHref(attrs.Href) + raw, err := json.Marshal(attrs) + if err != nil { + m.Attrs = []byte(`{"href":"#"}`) + return + } + + m.Attrs = raw +} diff --git a/pkg/prosemirror/sanitize_test.go b/pkg/prosemirror/sanitize_test.go new file mode 100644 index 000000000..696fc58e4 --- /dev/null +++ b/pkg/prosemirror/sanitize_test.go @@ -0,0 +1,84 @@ +// Copyright (c) 2026 Probo Inc . +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR +// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +package prosemirror + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSanitizeDocumentJSON_EmptyUnchanged(t *testing.T) { + t.Parallel() + + got, err := SanitizeDocumentJSON("") + require.NoError(t, err) + assert.Equal(t, "", got) + + got, err = SanitizeDocumentJSON(" ") + require.NoError(t, err) + assert.Equal(t, " ", got) +} + +func TestSanitizeDocumentJSON_NonJSONError(t *testing.T) { + t.Parallel() + + _, err := SanitizeDocumentJSON("plain text is not valid document JSON") + require.Error(t, err) +} + +func TestSanitizeDocumentJSON_NonDocRootError(t *testing.T) { + t.Parallel() + + _, err := SanitizeDocumentJSON(`{"type":"paragraph","content":[]}`) + require.Error(t, err) +} + +func TestSanitizeDocumentJSON_LinkHref(t *testing.T) { + t.Parallel() + + raw := `{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","marks":[{"type":"link","attrs":{"href":"javascript:alert(1)","target":"_blank"}}],"text":"click"}]}]}` + + out, err := SanitizeDocumentJSON(raw) + require.NoError(t, err) + + var doc Node + require.NoError(t, json.Unmarshal([]byte(out), &doc)) + txt := doc.Content[0].Content[0] + require.Len(t, txt.Marks, 1) + attrs, err := txt.Marks[0].LinkAttrs() + require.NoError(t, err) + assert.Equal(t, "#", attrs.Href) + require.NotNil(t, attrs.Target) + assert.Equal(t, "_blank", *attrs.Target) +} + +func TestSanitizeDocumentJSON_PreservesSafeHref(t *testing.T) { + t.Parallel() + + raw := `{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","marks":[{"type":"link","attrs":{"href":"https://example.com"}}],"text":"ok"}]}]}` + + out, err := SanitizeDocumentJSON(raw) + require.NoError(t, err) + + var doc Node + require.NoError(t, json.Unmarshal([]byte(out), &doc)) + txt := doc.Content[0].Content[0] + attrs, err := txt.Marks[0].LinkAttrs() + require.NoError(t, err) + assert.Equal(t, "https://example.com", attrs.Href) +} diff --git a/pkg/server/api/connect/v1/v1_resolver.go b/pkg/server/api/connect/v1/v1_resolver.go index 25d39a2e9..23d6b6b6e 100644 --- a/pkg/server/api/connect/v1/v1_resolver.go +++ b/pkg/server/api/connect/v1/v1_resolver.go @@ -1,17 +1,3 @@ -// Copyright (c) 2025-2026 Probo Inc . -// -// Permission to use, copy, modify, and/or distribute this software for any -// purpose with or without fee is hereby granted, provided that the above -// copyright notice and this permission notice appear in all copies. -// -// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH -// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY -// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, -// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM -// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR -// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR -// PERFORMANCE OF THIS SOFTWARE. - package connect_v1 // This file will be automatically regenerated based on the schema, any resolver diff --git a/pkg/server/api/console/v1/schema.graphql b/pkg/server/api/console/v1/schema.graphql index f0fd5469e..59342eb49 100644 --- a/pkg/server/api/console/v1/schema.graphql +++ b/pkg/server/api/console/v1/schema.graphql @@ -3766,7 +3766,6 @@ type Mutation { updateDocument(input: UpdateDocumentInput!): UpdateDocumentPayload! archiveDocument(input: ArchiveDocumentInput!): ArchiveDocumentPayload! unarchiveDocument(input: UnarchiveDocumentInput!): UnarchiveDocumentPayload! - updateDocumentVersionContent(input: UpdateDocumentVersionContentInput!): UpdateDocumentVersionContentPayload! deleteDocument(input: DeleteDocumentInput!): DeleteDocumentPayload! # Meeting mutations createMeeting(input: CreateMeetingInput!): CreateMeetingPayload! @@ -4486,11 +4485,6 @@ input UpdateDocumentInput { trustCenterVisibility: TrustCenterVisibility } -input UpdateDocumentVersionContentInput { - id: ID! - content: String! -} - input ExportDocumentVersionPDFInput { documentVersionId: ID! withWatermark: Boolean! @@ -5256,10 +5250,6 @@ type UnarchiveDocumentPayload { document: Document! } -type UpdateDocumentVersionContentPayload { - content: String! -} - type DeleteDocumentPayload { deletedDocumentId: ID! } diff --git a/pkg/server/api/console/v1/v1_resolver.go b/pkg/server/api/console/v1/v1_resolver.go index bbb0c825e..eca97c6a7 100644 --- a/pkg/server/api/console/v1/v1_resolver.go +++ b/pkg/server/api/console/v1/v1_resolver.go @@ -4734,38 +4734,6 @@ func (r *mutationResolver) UnarchiveDocument(ctx context.Context, input types.Un }, nil } -// UpdateDocumentVersionContent is the resolver for the updateDocumentVersionContent field. -func (r *mutationResolver) UpdateDocumentVersionContent(ctx context.Context, input types.UpdateDocumentVersionContentInput) (*types.UpdateDocumentVersionContentPayload, error) { - if err := r.authorize(ctx, input.ID, probo.ActionDocumentUpdate); err != nil { - return nil, err - } - - prb := r.ProboService(ctx, input.ID.TenantID()) - - content, err := prb.Documents.UpdateDocumentVersionContent( - ctx, - probo.UpdateDocumentVersionRequest{ - ID: input.ID, - Content: input.Content, - }, - ) - - if err != nil { - if _, ok := errors.AsType[*probo.ErrDocumentVersionNotDraft](err); ok { - return nil, gqlutils.Conflict(ctx, err) - } - if validationErrors, ok := errors.AsType[validator.ValidationErrors](err); ok { - return nil, gqlutils.InvalidValidationErrors(ctx, validationErrors) - } - r.logger.ErrorCtx(ctx, "cannot update document version content", log.Error(err)) - return nil, gqlutils.Internal(ctx) - } - - return &types.UpdateDocumentVersionContentPayload{ - Content: content, - }, nil -} - // DeleteDocument is the resolver for the deleteDocument field. func (r *mutationResolver) DeleteDocument(ctx context.Context, input types.DeleteDocumentInput) (*types.DeleteDocumentPayload, error) { if err := r.authorize(ctx, input.DocumentID, probo.ActionDocumentDelete); err != nil { diff --git a/pkg/server/api/mcp/v1/schema.resolvers.go b/pkg/server/api/mcp/v1/schema.resolvers.go index 104665e57..2e0253989 100644 --- a/pkg/server/api/mcp/v1/schema.resolvers.go +++ b/pkg/server/api/mcp/v1/schema.resolvers.go @@ -3351,29 +3351,3 @@ func (r *Resolver) PublishMinorDocumentVersionTool(ctx context.Context, req *mcp DocumentVersion: types.NewDocumentVersion(documentVersion), }, nil } - -func (r *Resolver) UpdateDocumentVersionContentTool(ctx context.Context, req *mcp.CallToolRequest, input *types.UpdateDocumentVersionContentInput) (*mcp.CallToolResult, types.UpdateDocumentVersionContentOutput, error) { - r.MustAuthorize(ctx, input.DocumentVersionID, probo.ActionDocumentUpdate) - - svc := r.ProboService(ctx, input.DocumentVersionID) - - content, err := svc.Documents.UpdateDocumentVersionContent( - ctx, - probo.UpdateDocumentVersionRequest{ - ID: input.DocumentVersionID, - Content: input.Content, - }, - ) - if err != nil { - var errNotDraft *probo.ErrDocumentVersionNotDraft - if errors.As(err, &errNotDraft) { - return nil, types.UpdateDocumentVersionContentOutput{}, fmt.Errorf("cannot update document version content: %w", err) - } - - panic(fmt.Errorf("cannot update document version content: %w", err)) - } - - return nil, types.UpdateDocumentVersionContentOutput{ - Content: content, - }, nil -} diff --git a/pkg/server/api/mcp/v1/specification.yaml b/pkg/server/api/mcp/v1/specification.yaml index 6f8376cf0..93557d4ff 100644 --- a/pkg/server/api/mcp/v1/specification.yaml +++ b/pkg/server/api/mcp/v1/specification.yaml @@ -5597,28 +5597,6 @@ components: document_version: $ref: "#/components/schemas/DocumentVersion" - UpdateDocumentVersionContentInput: - type: object - required: - - document_version_id - - content - properties: - document_version_id: - $ref: "#/components/schemas/GID" - description: Document version ID - content: - type: string - description: ProseMirror JSON document content - - UpdateDocumentVersionContentOutput: - type: object - required: - - content - properties: - content: - type: string - description: Updated document content - DeleteDraftDocumentVersionInput: type: object required: @@ -7506,14 +7484,6 @@ tools: $ref: "#/components/schemas/UpdateDocumentVersionInput" outputSchema: $ref: "#/components/schemas/UpdateDocumentVersionOutput" - - name: updateDocumentVersionContent - description: Update draft document version body content only (ProseMirror JSON). - hints: - readonly: false - inputSchema: - $ref: "#/components/schemas/UpdateDocumentVersionContentInput" - outputSchema: - $ref: "#/components/schemas/UpdateDocumentVersionContentOutput" - name: deleteDraftDocumentVersion description: Delete a draft document version hints: diff --git a/pkg/server/api/trust/v1/v1_resolver.go b/pkg/server/api/trust/v1/v1_resolver.go index ec6ed4cbb..7593974ae 100644 --- a/pkg/server/api/trust/v1/v1_resolver.go +++ b/pkg/server/api/trust/v1/v1_resolver.go @@ -1,17 +1,3 @@ -// Copyright (c) 2025-2026 Probo Inc . -// -// Permission to use, copy, modify, and/or distribute this software for any -// purpose with or without fee is hereby granted, provided that the above -// copyright notice and this permission notice appear in all copies. -// -// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH -// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY -// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, -// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM -// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR -// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR -// PERFORMANCE OF THIS SOFTWARE. - package trust_v1 // This file will be automatically regenerated based on the schema, any resolver diff --git a/pkg/validator/validator_prosemirror.go b/pkg/validator/validator_prosemirror.go new file mode 100644 index 000000000..151cb8170 --- /dev/null +++ b/pkg/validator/validator_prosemirror.go @@ -0,0 +1,40 @@ +// Copyright (c) 2026 Probo Inc . +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR +// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +package validator + +import ( + "strings" + + "go.probo.inc/probo/pkg/prosemirror" +) + +// ProseMirrorDocumentContent requires non-empty string values to be valid +// ProseMirror/Tiptap JSON with root type "doc". Empty and whitespace-only +// strings are allowed. +func ProseMirrorDocumentContent() ValidatorFunc { + return func(value any) *ValidationError { + s, ok := value.(string) + if !ok { + return newValidationError(ErrorCodeInvalidFormat, "value must be a string") + } + if strings.TrimSpace(s) == "" { + return nil + } + if err := prosemirror.ValidateDocumentContentJSON(s); err != nil { + return newValidationError(ErrorCodeInvalidFormat, err.Error()) + } + return nil + } +} diff --git a/pkg/validator/validator_prosemirror_test.go b/pkg/validator/validator_prosemirror_test.go new file mode 100644 index 000000000..6e94e7a7b --- /dev/null +++ b/pkg/validator/validator_prosemirror_test.go @@ -0,0 +1,51 @@ +// Copyright (c) 2026 Probo Inc . +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR +// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +package validator + +import "testing" + +func TestProseMirrorDocumentContent(t *testing.T) { + t.Parallel() + + validDoc := `{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"hi"}]}]}` + + tests := []struct { + name string + value any + wantError bool + }{ + {"empty string", "", false}, + {"whitespace only", " \n\t ", false}, + {"valid doc", validDoc, false}, + {"plain text", "not json", true}, + {"non-doc root", `{"type":"paragraph","content":[]}`, true}, + {"non-string", 1, true}, + } + + fn := ProseMirrorDocumentContent() + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := fn(tt.value) + if (err != nil) != tt.wantError { + t.Errorf("ProseMirrorDocumentContent() error = %v, wantError %v", err, tt.wantError) + } + if err != nil && err.Code != ErrorCodeInvalidFormat { + t.Errorf("expected code %s, got %s", ErrorCodeInvalidFormat, err.Code) + } + }) + } +}