diff --git a/.opencode/skills/gh-issue-from-pr/SKILL.md b/.opencode/skills/gh-issue-from-pr/SKILL.md new file mode 100644 index 0000000000..93f17345f2 --- /dev/null +++ b/.opencode/skills/gh-issue-from-pr/SKILL.md @@ -0,0 +1,230 @@ +--- +name: gh-issue-from-pr +description: Create a user-facing GitHub issue from a PR, separating the WHAT from the HOW, with correct milestone, project, labels, and issue type. +--- + +# Skill: gh-issue-from-pr + +Create a GitHub issue that captures the **WHAT** (user-facing feature or +bug) from an existing PR that describes the **HOW** (implementation). +Used when the project board needs an issue as the primary changelog/release unit. + +## When to Use + +- Create a tracking issue from a PR for changelog purposes +- Extract the user-facing problem/feature from a PR's implementation details +- Assign milestone, project, labels, and issue type to a new issue derived from a PR + +## Prerequisites + +- `gh` CLI authenticated (`gh auth status`) +- Permission to create issues and edit PRs in the target repository + +## Workflow + +### 1. Understand the PR + +```bash +gh pr view --repo penpot/penpot \ + --json title,body,author,labels,baseRefName,mergedAt,state,milestone +``` + +Identify: + +- **WHAT** — user-facing problem or feature. Goes into the issue. + Describe symptoms and impact, not internal mechanisms. +- **HOW** — implementation details. These belong in the PR, not the issue. + +### 2. Determine metadata + +| Field | Source | Rule | +|-------|--------|------| +| **Title** | PR title | Rewrite from user perspective. Strip leading emoji prefixes (`:bug:`, `:sparkles:`, `:tada:`). Focus on observable behavior. Use imperative mood. | +| **Labels** | PR labels | Copy user-facing labels (`bug`, `enhancement`, `community contribution`). Skip workflow labels (`backport candidate`, `team-qa`). | +| **Milestone** | PR milestone | **Always copy what's on the PR.** Fetch with: `gh pr view --json milestone --jq '.milestone.title'` If the PR has no milestone, create the issue without one. | +| **Project** | Always `Main` | Penpot uses the `Main` project (number 8) for all issues. | +| **Body** | PR's user-facing section | Extract steps to reproduce or feature description. Omit internal details. Use templates below. | +| **Issue Type** | PR labels / title | Map: `bug` label or `:bug:` title → `Bug`. `enhancement` label or `:sparkles:` title → `Enhancement`. Feature/epic → `Feature`. Default → `Task`. | + +### 3. Write the issue body + +**Bug template:** + +```markdown +### Description + + + +### Steps to reproduce + +1. +2. + +### Expected behavior + + + +### Affected versions + + +``` + +**Enhancement template:** + +```markdown +### Description + + + +### Use case + + + +### Affected versions + + +``` + +### 4. Create the issue + +Write the body to a temp file to avoid shell quoting issues: + +```bash +cat > /tmp/issue-body.md << 'ISSUE_BODY' + +ISSUE_BODY +``` + +Create: + +```bash +gh issue create \ + --repo penpot/penpot \ + --title "" \ + --label "<label1>" \ + --label "<label2>" \ + --milestone "<milestone>" \ + --project "Main" \ + --body-file /tmp/issue-body.md +``` + +Output: `https://github.com/penpot/penpot/issues/<NUMBER>` + +### 5. Assign to the PR author + +Assign the issue to the PR author so they're responsible for it: + +```bash +AUTHOR=$(gh pr view <PR_NUMBER> --repo penpot/penpot --json author --jq '.author.login') +gh issue edit <ISSUE_NUMBER> --repo penpot/penpot --add-assignee "$AUTHOR" +``` + +### 6. Set the Issue Type + +`gh issue create` can't set the Issue Type directly. Use GraphQL. + +Get the issue's GraphQL node ID: + +```bash +ISSUE_ID=$(gh api graphql -f query=' +query { repository(owner: "penpot", name: "penpot") { + issue(number: <ISSUE_NUMBER>) { id } +}}' --jq '.data.repository.issue.id') +``` + +Issue Type IDs for the Penpot repo: + +| Type | ID | +|------|----| +| Bug | `IT_kwDOAcyBPM4AX5Nb` | +| Enhancement | `IT_kwDOAcyBPM4B_IQN` | +| Feature | `IT_kwDOAcyBPM4AX5Nf` | +| Task | `IT_kwDOAcyBPM4AX5NY` | +| Question | `IT_kwDOAcyBPM4B_IQj` | +| Docs | `IT_kwDOAcyBPM4B_IQz` | + +Set it: + +```bash +gh api graphql -f query=' +mutation { + updateIssue(input: { + id: "'"$ISSUE_ID"'" + issueTypeId: "<TYPE_ID>" + }) { + issue { number issueType { name } } + } +}' +``` + +### 7. Verify + +```bash +gh issue view <ISSUE_NUMBER> --repo penpot/penpot \ + --json title,milestone,projectItems,labels \ + --jq '{title, milestone: .milestone.title, projects: [.projectItems[].title], labels: [.labels[].name]}' + +gh api graphql -f query=' +query { repository(owner: "penpot", name: "penpot") { + issue(number: <ISSUE_NUMBER>) { issueType { name } } +}}' --jq '.data.repository.issue.issueType.name' +``` + +### 8. Link the PR to the issue + +Append `Fixes #<ISSUE_NUMBER>` to the PR body: + +```bash +gh pr view <PR_NUMBER> --repo penpot/penpot --json body --jq '.body' > /tmp/pr-body.md +printf "\n\nFixes #<ISSUE_NUMBER>\n" >> /tmp/pr-body.md +gh pr edit <PR_NUMBER> --repo penpot/penpot --body-file /tmp/pr-body.md + +# Verify +gh pr view <PR_NUMBER> --repo penpot/penpot --json body \ + --jq '.body | test("Fixes #<ISSUE_NUMBER>")' +``` + +**Note:** If the PR is already merged, `Fixes` won't auto-close the issue +— it only creates the "Development" sidebar link. This is the desired +behavior since the issue is a tracking artifact. + +### 9. Clean up + +```bash +rm -f /tmp/issue-body.md /tmp/pr-body.md +``` + +## Label rules + +| PR has | Issue gets | +|--------|-----------| +| `bug` | `bug` | +| `enhancement` | `enhancement` | +| `community contribution` | `community contribution` | +| `backport candidate` | *(skip — workflow label)* | +| `team-qa` | *(skip — workflow label)* | +| No user-facing label | Infer from title: `:bug:` → `bug`, `:sparkles:` → `enhancement` | + +## Issue Type mapping + +| PR label(s) / title prefix | Issue Type | +|----------------------------|-----------| +| `bug` or `:bug:` | Bug | +| `enhancement` or `:sparkles:` or `:tada:` | Enhancement | +| Feature / epic | Feature | +| Documentation | Docs | +| None of the above | Task | + +## Key Principles + +- **Issue = WHAT, PR = HOW.** Never put implementation details in the + issue body. The issue is for users, QA, and changelog readers. +- **Copy the milestone from the PR.** Don't guess based on branch names. + If the PR has no milestone, create the issue without one. +- **Set Issue Type via GraphQL** — `gh issue create` can't set it. +- **Link via PR body** — `Fixes #<NUMBER>` creates the "Development" + sidebar link automatically. +- **One issue per PR** — even if a PR fixes multiple things, create a + single issue that summarizes the overall change. +- **Community attribution:** if the PR has the `community contribution` + label or the author is not a core team member, add the label to the issue. diff --git a/.opencode/skills/update-changelog/SKILL.md b/.opencode/skills/update-changelog/SKILL.md new file mode 100644 index 0000000000..889e77c3e6 --- /dev/null +++ b/.opencode/skills/update-changelog/SKILL.md @@ -0,0 +1,201 @@ +--- +name: update-changelog +description: Update the project CHANGES.md with issues from a given GitHub milestone, with correct categorization and references. +--- + +# Skill: update-changelog + +Update `CHANGES.md` with entries for all issues and PRs in a given GitHub +milestone. Each entry references the user-facing issue (not the PR) as the +primary link, with the fix PR on a sub-line. + +## When to Use + +- Before a new release, to populate the changelog with all fixed issues +- When new issues are added to an existing milestone and the changelog needs + to be refreshed +- To ensure every entry follows the correct format for the changelog + +## Prerequisites + +- `gh` CLI authenticated (`gh auth status`) +- Read access to the penpot/penpot repository + +## Workflow + +### 1. Determine the target version + +The version is typically a semver string like `2.15.3`. Confirm with the user +if not specified. + +### 2. Fetch all issues and PRs in the milestone + +Find the milestone number: + +```bash +gh api repos/penpot/penpot/milestones --paginate \ + --jq '.[] | select(.title=="<VERSION>") | {number: .number, title: .title, open_issues: .open_issues, closed_issues: .closed_issues}' +``` + +Then fetch all items: + +```bash +MILESTONE_NUMBER=<NUMBER> +gh api "repos/penpot/penpot/issues?milestone=$MILESTONE_NUMBER&state=all&per_page=100" \ + --jq '.[] | {number: .number, title: .title, state: .state, labels: [.labels[].name], pull_request: .pull_request != null}' +``` + +### 3. Identify issue ↔ PR relationships + +For each item, determine the relationship: + +- **Issue** (`pull_request: false`): This is the user-facing issue. It + becomes the primary link in the changelog. +- **PR** (`pull_request: true`): Check if it has `Fixes #<NUMBER>` in its + body to find which issue it closes. + +To find the linked issue for a PR: + +```bash +gh pr view <PR_NUMBER> --repo penpot/penpot \ + --json body,closingIssuesReferences --jq '{closingIssues: [.closingIssuesReferences[].number]}' +``` + +**Only closed issues are included.** An issue must have `state: "closed"` to +appear in the changelog. Open/unresolved issues are omitted, even if they are +tracked in the milestone. + +**Pairing rules:** + +| Pattern | Changelog format | +|---------|-----------------| +| Closed issue + one or more PRs fix it | Primary link = issue, sub-line with PRs comma-separated | +| PR exists with no linked issue | If a corresponding closed issue exists in the same milestone, link the issue. Otherwise, skip the entry (the issue must be the changelog unit). | +| Closed issue with no fix PR in milestone | Link the issue directly, without a PR sub-line. | + +### 4. Categorize entries + +Check the labels on each issue/PR: + +```bash +gh issue view <NUMBER> --repo penpot/penpot --json labels --jq '[.labels[].name]' +``` + +| Label / Title prefix | Changelog section | +|----------------------|-------------------| +| `bug` label or `:bug:` title prefix | `### :bug: Bugs fixed` | +| `enhancement` label or `:sparkles:` prefix | `### :sparkles: New features & Enhancements` | +| No label | Infer from title convention, default to bug fix | + +**Community contribution attribution:** If the issue or its fix PR has the +`community contribution` label, add an attribution `(by @<github_username>)` +on the changelog entry line, **before** the GitHub issue/PR references. +Fetch the author: + +```bash +gh issue view <NUMBER> --repo penpot/penpot --json author --jq '.author.login' +``` + +Placement in the entry line: +```markdown +- Fix description of the bug (by @username) [Github #<ISSUE>](...) + (PR: [#<PR>](...)) +``` + +### 5. Read the current CHANGES.md + +Read the top of `CHANGES.md` to understand the existing format and find the +insertion point (newest version goes at the top, after the `# CHANGELOG` +header). + +Key format rules from the existing file: + +```markdown +## <VERSION> + +### :bug: Bugs fixed + +- Fix description of the bug [Github #<ISSUE>](https://github.com/penpot/penpot/issues/<ISSUE>) + (PR: [#<PR>](https://github.com/penpot/penpot/pull/<PR>)) +- Fix another bug (by @contributor) [Github #<ISSUE>](https://github.com/penpot/penpot/issues/<ISSUE>) + (PR: [#<PR>](https://github.com/penpot/penpot/pull/<PR>)) + +### :sparkles: New features & Enhancements + +- Add new feature description [Github #<ISSUE>](https://github.com/penpot/penpot/issues/<ISSUE>) + (PR: [#<PR>](https://github.com/penpot/penpot/pull/<PR>)) +``` + +Format details: +- Entries start with `- ` followed by a short description in imperative mood +- Primary link is **always the issue** (user-facing artifact) +- PR references are on an indented sub-line: ` (PR: [#<N>](<url>))` + If an issue has multiple fix PRs, they are comma-separated on one line: + ` (PR: [#<N>](<url>), [#<M>](<url>))` +- The description should describe the fix/feature from the user's perspective +- Community contributions get `(by @<username>)` **before** the GitHub link +- Sections are separated by a blank line between the last entry and the next + section title +- Only include a section if there are entries for it + +### 6. Build the description text + +Derive the description from the issue title, not the PR title. Strip leading +emoji prefixes (`:bug:`, `:sparkles:`, `:tada:`) and focus on the +user-facing behavior. + +Examples: + +| Issue title | Changelog description | +|-------------|----------------------| +| `Plugin API token methods fail with schema validation error on PRO` | `Fix Plugin API token methods failing with schema validation error on PRO` | +| `Comment content is not sanitized before rendering, enabling stored XSS` | `Sanitize comment content on rendering` | +| `Custom uploaded font family names are not sanitized` | `Sanitize font family names on custom uploaded fonts` | + +### 7. Insert the section into CHANGES.md + +Insert the new version section right after the `# CHANGELOG` header (before +the previous version entry). Use the `edit` tool with enough context to make +a unique match. + +### 8. Verify + +Read the top of `CHANGES.md` and confirm: +- The version header is correct +- Every entry has a GitHub link +- Entries with a fix PR have the PR sub-line +- The section ordering is correct (newest first) +- Formatting matches the surrounding entries + +## Version section template + +```markdown +## <VERSION> + +### :bug: Bugs fixed + +- <fix description> [Github #<ISSUE>](https://github.com/penpot/penpot/issues/<ISSUE>) + (PR: [#<PR>](https://github.com/penpot/penpot/pull/<PR>)) +- <fix description> (by @contributor) [Github #<ISSUE>](https://github.com/penpot/penpot/issues/<ISSUE>) + (PR: [#<PR>](https://github.com/penpot/penpot/pull/<PR>)) +``` + +## Key Principles + +- **Issue = changelog unit.** The primary link always points to the + user-facing issue, not the implementation PR. +- **PR = implementation detail.** Reference the PR on a sub-line so readers + can find the code changes. +- **Latest version first.** New sections are inserted at the top of the + changelog, below the `# CHANGELOG` header. +- **User-facing descriptions.** Write from the user's perspective — describe + what broke and what was fixed, not internal implementation details. +- **Community attribution.** When the issue or fix PR has the + `community contribution` label, add `(by @<username>)` on the entry line + between the description and the GitHub link. +- **Only closed issues.** An issue must have `state: "closed"` to appear in + the changelog. Open unresolved issues are omitted. +- **Multiple PRs per issue.** If multiple PRs fix the same issue, list them + comma-separated on the same sub-line: `(PR: [#A](url), [#B](url))`. +- **Re-fetch before editing.** Milestones can change — always re-fetch issues + before making edits, don't rely on cached data. diff --git a/CHANGES.md b/CHANGES.md index 664959ef9c..2ab1722db8 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -19,7 +19,7 @@ - Option to download custom fonts (by @dfelinto) [Github #8320](https://github.com/penpot/penpot/issues/8320) - Add copy as image to clipboard option to workspace context menu (by @dfelinto) [Github #8313](https://github.com/penpot/penpot/pull/8313) - Add Tab/Shift+Tab navigation to rename layers sequentially (by @bittoby) [Github #8474](https://github.com/penpot/penpot/pull/8474) -- Copy and paste entire rows in existing table (by @bittoby) [Github #8498](https://github.com/penpot/penpot/pull/8498) +- Copy and paste entire rows in existing table (by @bittoby) [Github #8498](https://github.com/penpot/penpot/pull/84r98) - Rename token group [Taiga #13137](https://tree.taiga.io/project/penpot/us/13137) - Duplicate token group [Taiga #10653](https://tree.taiga.io/project/penpot/us/10653) - Copy token name from contextual menu [Taiga #13568](https://tree.taiga.io/project/penpot/issue/13568) @@ -126,6 +126,17 @@ - Fix library updates reappear after being applied and the file is reloaded [Taiga #14040](https://tree.taiga.io/project/penpot/issue/14040) - Fix dependency libraries remaining visible in UI after unlinking main library [Taiga #14020](https://tree.taiga.io/project/penpot/issue/14020) +## 2.15.3 + +### :bug: Bugs fixed + +- Fix Plugin API token methods failing with schema validation error on PRO [Github #9641](https://github.com/penpot/penpot/issues/9641) + (PR: [#9632](https://github.com/penpot/penpot/pull/9632)) +- Sanitize comment content on rendering [Github #9642](https://github.com/penpot/penpot/issues/9642) + (PR: [#9605](https://github.com/penpot/penpot/pull/9605)) +- Sanitize font family names on custom uploaded fonts [Github #9643](https://github.com/penpot/penpot/issues/9643) + (PR: [#9601](https://github.com/penpot/penpot/pull/9601)) + ## 2.15.2 ### :bug: Bugs fixed diff --git a/backend/src/app/rpc/commands/fonts.clj b/backend/src/app/rpc/commands/fonts.clj index a5ff63c0e1..485d41b571 100644 --- a/backend/src/app/rpc/commands/fonts.clj +++ b/backend/src/app/rpc/commands/fonts.clj @@ -13,6 +13,7 @@ [app.common.media :as cm] [app.common.schema :as sm] [app.common.time :as ct] + [app.common.types.font :as types.font] [app.common.uuid :as uuid] [app.db :as db] [app.db.sql :as-alias sql] @@ -99,7 +100,7 @@ [:map {:title "create-font-variant"} [:team-id ::sm/uuid] [:font-id ::sm/uuid] - [:font-family ::sm/text] + [:font-family types.font/schema:font-family] [:font-weight [::sm/one-of {:format "number"} valid-weight]] [:font-style [::sm/one-of {:format "string"} valid-style]] [:data {:optional true} [:map-of ::sm/text [:or ::sm/bytes [::sm/vec ::sm/bytes]]]] @@ -277,7 +278,7 @@ [:map {:title "update-font"} [:team-id ::sm/uuid] [:id ::sm/uuid] - [:name :string]]) + [:name types.font/schema:font-family]]) (sv/defmethod ::update-font {::doc/added "1.18" diff --git a/backend/test/backend_tests/rpc_font_test.clj b/backend/test/backend_tests/rpc_font_test.clj index 59a58466b9..8733ca3ad9 100644 --- a/backend/test/backend_tests/rpc_font_test.clj +++ b/backend/test/backend_tests/rpc_font_test.clj @@ -825,3 +825,100 @@ (t/is (some? (:error out))) (t/is (= :validation (-> out :error ex-data :type))) (t/is (= :media-type-not-allowed (-> out :error ex-data :code)))))) + +;; --- Font family name validation / XSS prevention + +(t/deftest create-font-variant-with-invalid-family + (with-mocks [mock {:target 'app.rpc.quotes/check! :return nil}] + (let [prof (th/create-profile* 1 {:is-active true}) + team-id (:default-team-id prof) + font-id (uuid/custom 10 100) + data (-> (io/resource "backend_tests/test_files/font-1.ttf") (io/read*))] + + ;; name with < should fail + (let [params {::th/type :create-font-variant + ::rpc/profile-id (:id prof) + :team-id team-id :font-id font-id + :font-family "evil<script>alert(1)</script>" + :font-weight 400 :font-style "normal" + :data {"font/ttf" data}} + out (th/command! params)] + (t/is (not (th/success? out))) + (t/is (th/ex-of-type? (:error out) :validation)) + (t/is (th/ex-of-code? (:error out) :params-validation))) + + ;; name with ' should fail + (let [params {::th/type :create-font-variant + ::rpc/profile-id (:id prof) + :team-id team-id :font-id font-id + :font-family "evil'name" + :font-weight 400 :font-style "normal" + :data {"font/ttf" data}} + out (th/command! params)] + (t/is (not (th/success? out))) + (t/is (th/ex-of-type? (:error out) :validation))) + + ;; name with } should fail + (let [params {::th/type :create-font-variant + ::rpc/profile-id (:id prof) + :team-id team-id :font-id font-id + :font-family "evil}name" + :font-weight 400 :font-style "normal" + :data {"font/ttf" data}} + out (th/command! params)] + (t/is (not (th/success? out))) + (t/is (th/ex-of-type? (:error out) :validation))) + + ;; valid name should succeed + (let [params {::th/type :create-font-variant + ::rpc/profile-id (:id prof) + :team-id team-id :font-id (uuid/custom 10 101) + :font-family "Source Sans Pro" + :font-weight 400 :font-style "normal" + :data {"font/ttf" data}} + out (th/command! params)] + (t/is (th/success? out)))))) + +(t/deftest update-font-with-invalid-family + (with-mocks [mock {:target 'app.rpc.quotes/check! :return nil}] + (let [prof (th/create-profile* 1 {:is-active true}) + team-id (:default-team-id prof) + font-id (uuid/custom 10 102) + data (-> (io/resource "backend_tests/test_files/font-1.ttf") (io/read*))] + + ;; Create a valid font first + (let [params {::th/type :create-font-variant + ::rpc/profile-id (:id prof) + :team-id team-id :font-id font-id + :font-family "ValidFont" + :font-weight 400 :font-style "normal" + :data {"font/ttf" data}} + out (th/command! params)] + (t/is (th/success? out))) + + ;; rename with < should fail + (let [params {::th/type :update-font + ::rpc/profile-id (:id prof) + :team-id team-id :id font-id + :name "evil<script>x</script>"} + out (th/command! params)] + (t/is (not (th/success? out))) + (t/is (th/ex-of-type? (:error out) :validation)) + (t/is (th/ex-of-code? (:error out) :params-validation))) + + ;; rename with ' should fail + (let [params {::th/type :update-font + ::rpc/profile-id (:id prof) + :team-id team-id :id font-id + :name "evil'name"} + out (th/command! params)] + (t/is (not (th/success? out))) + (t/is (th/ex-of-type? (:error out) :validation))) + + ;; valid rename should succeed + (let [params {::th/type :update-font + ::rpc/profile-id (:id prof) + :team-id team-id :id font-id + :name "Valid Font Name"} + out (th/command! params)] + (t/is (th/success? out)))))) diff --git a/common/src/app/common/types/font.cljc b/common/src/app/common/types/font.cljc new file mode 100644 index 0000000000..61ee5a6059 --- /dev/null +++ b/common/src/app/common/types/font.cljc @@ -0,0 +1,21 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC + +(ns app.common.types.font + (:require + [app.common.schema :as sm])) + +(def ^:private font-family-re + ;; \p{L} (Unicode letter) works in Java regex natively, but in JavaScript it + ;; requires the "u" flag which ClojureScript regex literals don't support. + #?(:clj #"[\p{L}\d _.-]+" + :cljs (js/RegExp. "[\\p{L}\\d _.-]+" "u"))) + +(def schema:font-family + [:and + [::sm/text {:max 250}] + [:fn {:error/code "errors.font-family-invalid-chars"} + (fn [s] (boolean (re-matches font-family-re s)))]]) diff --git a/common/test/common_tests/types/font_test.cljc b/common/test/common_tests/types/font_test.cljc new file mode 100644 index 0000000000..ee04e656fb --- /dev/null +++ b/common/test/common_tests/types/font_test.cljc @@ -0,0 +1,41 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC + +(ns common-tests.types.font-test + (:require + [app.common.schema :as sm] + [app.common.types.font :as ctf] + [clojure.test :as t])) + +(t/deftest font-family-schema-valid + (t/is (sm/validate ctf/schema:font-family "Source Sans Pro")) + (t/is (sm/validate ctf/schema:font-family "Roboto")) + (t/is (sm/validate ctf/schema:font-family "Open Sans 300")) + (t/is (sm/validate ctf/schema:font-family "Font-Name_v2")) + (t/is (sm/validate ctf/schema:font-family "Noto Sans CJK SC")) + (t/is (sm/validate ctf/schema:font-family "A")) + ;; hyphens, underscores and dots are allowed + (t/is (sm/validate ctf/schema:font-family "Fira-Code")) + (t/is (sm/validate ctf/schema:font-family "font_name")) + (t/is (sm/validate ctf/schema:font-family "Soucre Sans Pro 3.0")) + ;; Unicode letters are allowed + (t/is (sm/validate ctf/schema:font-family "思源黑体")) + (t/is (sm/validate ctf/schema:font-family "العربية"))) + +(t/deftest font-family-schema-invalid + ;; HTML injection characters + (t/is (not (sm/validate ctf/schema:font-family "evil<script>"))) + (t/is (not (sm/validate ctf/schema:font-family "<test>name"))) + ;; CSS injection characters + (t/is (not (sm/validate ctf/schema:font-family "evil'name"))) + (t/is (not (sm/validate ctf/schema:font-family "evil\"name"))) + (t/is (not (sm/validate ctf/schema:font-family "evil}name"))) + (t/is (not (sm/validate ctf/schema:font-family "evil;name"))) + (t/is (not (sm/validate ctf/schema:font-family "evil\\name"))) + ;; empty string + (t/is (not (sm/validate ctf/schema:font-family ""))) + ;; too long + (t/is (not (sm/validate ctf/schema:font-family (apply str (repeat 251 "a")))))) diff --git a/frontend/src/app/main/ui/comments.cljs b/frontend/src/app/main/ui/comments.cljs index 3093a11287..8ee83d7411 100644 --- a/frontend/src/app/main/ui/comments.cljs +++ b/frontend/src/app/main/ui/comments.cljs @@ -95,7 +95,7 @@ ([text] (-> (dom/create-element "span") (dom/set-data! "type" "text") - (dom/set-html! (if (empty? text) zero-width-space text))))) + (dom/set-html! (if (empty? text) zero-width-space (dom/escape-html text)))))) (defn- create-mention-node "Creates a mention node" @@ -334,7 +334,7 @@ after-span (create-text-node (dm/str " " suffix)) sel (wapi/get-selection)] - (dom/set-html! span-node (if (empty? prefix) zero-width-space prefix)) + (dom/set-html! span-node (if (empty? prefix) zero-width-space (dom/escape-html prefix))) (dom/insert-after! node span-node mention-span) (dom/insert-after! node mention-span after-span) (wapi/set-cursor-after! after-span) @@ -351,7 +351,7 @@ (let [node-text (dom/get-text span-node) at-symbol (if (blank-content? node-text) "@" " @")] - (dom/set-html! span-node (str/concat node-text at-symbol)) + (dom/set-html! span-node (str/concat (dom/escape-html node-text) at-symbol)) (wapi/set-cursor-after! span-node)))))) handle-key-down @@ -399,7 +399,7 @@ (when span-node (let [txt (.-textContent span-node)] - (dom/set-html! span-node (dm/str (subs txt 0 offset) "\n" zero-width-space (subs txt offset))) + (dom/set-html! span-node (dm/str (dom/escape-html (subs txt 0 offset)) "\n" zero-width-space (dom/escape-html (subs txt offset)))) (wapi/set-cursor! span-node (inc offset)) (handle-input))))) diff --git a/frontend/src/app/main/ui/dashboard/fonts.cljs b/frontend/src/app/main/ui/dashboard/fonts.cljs index 16b8c7dec3..db96f2a901 100644 --- a/frontend/src/app/main/ui/dashboard/fonts.cljs +++ b/frontend/src/app/main/ui/dashboard/fonts.cljs @@ -11,6 +11,8 @@ [app.common.data.macros :as dm] [app.common.exceptions :as ex] [app.common.media :as cm] + [app.common.schema :as sm] + [app.common.types.font :as ctf] [app.common.uuid :as uuid] [app.config :as cf] [app.main.data.fonts :as df] @@ -139,7 +141,8 @@ (dom/get-data "id") (uuid/parse)) name (dom/get-value target)] - (when-not (str/blank? name) + (when (and (not (str/blank? name)) + (sm/validate ctf/schema:font-family name)) (swap! fonts* df/rename-and-regroup id name installed-fonts))))) on-change-name @@ -320,7 +323,9 @@ (fn [_] (reset! edition* false) (when-not (str/blank? font-family) - (st/emit! (df/update-font {:id font-id :name font-family}))))) + (if (sm/validate ctf/schema:font-family font-family) + (st/emit! (df/update-font {:id font-id :name font-family})) + (st/emit! (ntf/error (tr "errors.font-family-invalid-chars"))))))) on-key-down (mf/use-fn diff --git a/frontend/src/app/util/dom.cljs b/frontend/src/app/util/dom.cljs index 2ce67382b1..1727603670 100644 --- a/frontend/src/app/util/dom.cljs +++ b/frontend/src/app/util/dom.cljs @@ -331,6 +331,18 @@ ([document ^js text] (.createTextNode document text))) +(defn escape-html + "Escapes special HTML characters in a string so that it can be safely used + as innerHTML without risk of XSS." + [^js text] + (when (some? text) + (-> text + (str/replace "&" "&") + (str/replace "<" "<") + (str/replace ">" ">") + (str/replace "\"" """) + (str/replace "'" "'")))) + (defn set-html! [^js el html] (when (some? el) diff --git a/frontend/translations/en.po b/frontend/translations/en.po index 989ba1c578..a363259cf1 100644 --- a/frontend/translations/en.po +++ b/frontend/translations/en.po @@ -1544,6 +1544,10 @@ msgstr "Invalid text" msgid "errors.team-name-invalid-chars" msgstr "The team name can't contain any of the following characters:'.', ':' or '/'" +#: common/src/app/common/types/font.cljc +msgid "errors.font-family-invalid-chars" +msgstr "The font family name can only contain letters, numbers, spaces, hyphens, underscores, and dots." + #: src/app/main/ui/static.cljs:74 msgid "errors.invite-invalid" msgstr "Invite invalid"