742 normalize text handling (#751)

* Remove input sanitization
* Move from hylo-utils to hylo-shared library
* Remove Post#detailsText
* Normalize link generation and formatting for notifications and emails
* Update savedSearch post body URLs to include group context
* Fix tests remove noisy test feedback
* Update dependencies

CHANGELOG.md

@@ -4,6 +4,14 @@ All notable changes to Hylo Node (the Hylo server) will be documented in this fi
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+
+### Changed
+- Move HTML sanitization exclusively of posts.details and comments.text to model getters (`Post#details()` and `Comment.text()`) which should be used EXCLUSIVELY for any output through API
+- Removes extraneous and destructive HTML sanitization on plain text fields (entity decoding and protection from related React text renders)
+- Adopts graphql-tools default getters for Post model to reduce duplication of code and make model results canonical for sanitization
+- Move from deprecated `hylo-utils` to new `hylo-shared` library
+
 ## [3.1.3] - 2022-02-22
 
 ### Added

api/controllers/CommentController.js

@@ -1,7 +1,7 @@
 /* eslint-disable camelcase */
 import { isEmpty } from 'lodash'
 import { flow, filter, map, includes } from 'lodash/fp'
-import { markdown } from 'hylo-utils/text'
+import { TextHelpers } from 'hylo-shared'
 import createComment from '../models/comment/createComment'
 
 module.exports = {
@@ -42,7 +42,7 @@ module.exports = {
  // TODO: fix
  const { groupId, userId } = res.locals.tokenData
 
- const replyText = postId => markdown(req.param(`post-${postId}`))
+ const replyText = postId => TextHelpers.markdown(req.param(`post-${postId}`))
 
  const postIds = flow(
  Object.keys,

api/graphql/filters.js

@@ -1,6 +1,3 @@
-import { curry } from 'lodash'
-import GroupDataType from '../models/group/DataType'
-
 export const commentFilter = userId => relation => relation.query(q => {
  q.distinct()
  q.where({'comments.active': true})

api/graphql/index.test.js

@@ -197,7 +197,7 @@ describe('graphql request handler', () => {
  comments: {
  items: [
  {
- text: comment.get('text'),
+ text: comment.text(),
  creator: {
  name: user2.get('name')
  }
@@ -276,7 +276,7 @@ describe('graphql request handler', () => {
  comments: {
  items: [
  {
- text: comment.get('text'),
+ text: comment.text(),
  attachments: [
  {
  id: media.id,

api/graphql/makeModels.js

@@ -10,14 +10,12 @@ import {
  postFilter,
  voteFilter
 } from './filters'
-import { flow, mapKeys, camelCase } from 'lodash/fp'
+import { mapKeys, camelCase } from 'lodash/fp'
 import InvitationService from '../services/InvitationService'
 import {
- filterAndSortGroups,
  filterAndSortPosts,
  filterAndSortUsers
 } from '../services/Search/util'
-import he from 'he';
 
 // this defines what subset of attributes and relations in each Bookshelf model
 // should be exposed through GraphQL, and what query filters should be applied
@@ -162,14 +160,8 @@ export default async function makeModels (userId, isAdmin) {
  'type'
  ],
  getters: {
- title: p => p.get('name') ? he.decode(p.get('name')) : null,
- details: p => p.get('description'),
- detailsText: p => p.getDetailsText(),
- isPublic: p => p.get('is_public'),
  commenters: (p, { first }) => p.getCommenters(first, userId),
  commentersTotal: p => p.getCommentersTotal(userId),
- commentsTotal: p => p.get('num_comments'),
- votesTotal: p => p.get('num_votes'),
  myVote: p => userId ? p.userVote(userId).then(v => !!v) : false,
  myEventResponse: p =>
  userId && p.isEvent() ? p.userEventInvitation(userId)

api/graphql/mutations/topic.js

@@ -1,5 +1,3 @@
-import { sanitize } from 'hylo-utils/text'
-
 export async function topicMutationPermissionCheck (userId, groupId) {
  const group = await Group.find(groupId)
  if (!group) {
@@ -12,13 +10,12 @@ export async function topicMutationPermissionCheck (userId, groupId) {
 
 export async function createTopic (userId, topicName, groupId, isDefault, isSubscribing = true) {
  await topicMutationPermissionCheck(userId, groupId)
- const name = sanitize(topicName)
- const invalidReason = Tag.validate(name)
+ const invalidReason = Tag.validate(topicName)
  if (invalidReason) {
  throw new Error(invalidReason)
  }
 
- const topic = await Tag.findOrCreate(name)
+ const topic = await Tag.findOrCreate(topicName)
  await Tag.addToGroup({
  group_id: groupId,
  tag_id: topic.id,

api/graphql/mutations/topic.test.js

@@ -33,7 +33,7 @@ describe('topic mutations', () => {
  })
 
  describe('createTopic', () => {
- // validation is mostly tested in hylo-utils, so this just left here to show willing...
+ // validation is mostly tested in hylo-shared, so this just left here to show willing...
  it('rejects on invalid topic names', async () => {
  const actual = mutations.createTopic(
  u1.id,

api/graphql/schema.graphql

@@ -402,7 +402,6 @@ type Post {
  id: ID
  title: String
  details: String
- detailsText: String
  type: String
  createdAt: Date
  updatedAt: Date

api/models/Comment.js

@@ -1,5 +1,4 @@
-/* eslint-disable camelcase */
-import { markdown } from 'hylo-utils/text'
+import { TextHelpers } from 'hylo-shared'
 import { notifyAboutMessage, sendDigests } from './comment/notifications'
 import EnsureLoad from './mixins/EnsureLoad'
 
@@ -17,7 +16,8 @@ module.exports = bookshelf.Model.extend(Object.assign({
  },
 
  text: function () {
- return this.get('text')
+ // This should be always used when accessing this attribute
+ return TextHelpers.sanitizeHTML(this.get('text'))
  },
 
  mentions: function () {
@@ -138,7 +138,7 @@ module.exports = bookshelf.Model.extend(Object.assign({
  })
 
  const finalText = cutoff ? lines.slice(0, cutoff).join('\n') : text
- return opts.useMarkdown ? markdown(finalText || '') : finalText
+ return opts.useMarkdown ? TextHelpers.markdown(finalText || '') : finalText
  },
 
  notifyAboutMessage,

api/models/FlaggedItem.js

@@ -1,5 +1,5 @@
 import { values, isEmpty, trim } from 'lodash'
-import { validateFlaggedItem } from 'hylo-utils/validators'
+import { Validators } from 'hylo-shared'
 import { notifyModeratorsPost, notifyModeratorsMember, notifyModeratorsComment } from './flaggedItem/notifyUtils'
 
 module.exports = bookshelf.Model.extend({
@@ -83,11 +83,11 @@ module.exports = bookshelf.Model.extend({
  reason = 'N/A'
  }
 
- const invalidReason = validateFlaggedItem.reason(reason)
+ const invalidReason = Validators.validateFlaggedItem.reason(reason)
  if (invalidReason) return Promise.reject(new Error(invalidReason))
 
  if (process.env.NODE_ENV !== 'development') {
- const invalidLink = validateFlaggedItem.link(link)
+ const invalidLink = Validators.validateFlaggedItem.link(link)
  if (invalidLink) return Promise.reject(new Error(invalidLink))
  }
 

api/models/GroupRelationshipInvite.js

@@ -1,4 +1,3 @@
-import uuid from 'node-uuid'
 import EnsureLoad from './mixins/EnsureLoad'
 
 module.exports = bookshelf.Model.extend(Object.assign({

api/models/LinkedAccount.js

@@ -1,8 +1,8 @@
 import bcrypt from 'bcrypt'
 import Promise from 'bluebird'
-import { get, isEmpty, merge, pick } from 'lodash'
+import { get, isEmpty } from 'lodash'
+import { Validators } from 'hylo-shared'
 const hash = Promise.promisify(bcrypt.hash, bcrypt)
-import { validateUser } from 'hylo-utils/validators'
 
 module.exports = bookshelf.Model.extend({
  tableName: 'linked_account',
@@ -29,7 +29,7 @@ module.exports = bookshelf.Model.extend({
 }, {
  create: function (userId, { type, profile, password, token }, { transacting, updateUser } = {}) {
  if (type === 'password') {
- const invalidReason = validateUser.password(password)
+ const invalidReason = Validators.validateUser.password(password)
  if (invalidReason) return Promise.reject(new Error(invalidReason))
  }
 

api/models/Notification.js

@@ -324,7 +324,7 @@ module.exports = bookshelf.Model.extend({
  var post = this.post()
  var reader = this.reader()
  var user = post.relations.user
- var description = RichText.qualifyLinks(post.get('description'))
+ var description = RichText.qualifyLinks(post.details())
  var replyTo = Email.postReplyAddress(post.id, reader.id)
 
  var groupIds = Activity.groupIds(this.relations.activity)
@@ -362,7 +362,7 @@ module.exports = bookshelf.Model.extend({
  var post = this.post()
  var reader = this.reader()
  var user = post.relations.user
- var description = RichText.qualifyLinks(post.get('description'))
+ var description = RichText.qualifyLinks(post.details())
  var replyTo = Email.postReplyAddress(post.id, reader.id)
 
  var groupIds = Activity.groupIds(this.relations.activity)
@@ -404,7 +404,7 @@ module.exports = bookshelf.Model.extend({
 
  const post = comment.relations.post
  const commenter = comment.relations.user
- const text = RichText.qualifyLinks(comment.get('text'))
+ const text = RichText.qualifyLinks(comment.text())
  const replyTo = Email.postReplyAddress(post.id, reader.id)
  const title = decode(post.get('name'))
 
@@ -653,7 +653,7 @@ module.exports = bookshelf.Model.extend({
  var post = this.post()
  var reader = this.reader()
  var inviter = this.actor()
- var description = RichText.qualifyLinks(post.get('description'))
+ var description = RichText.qualifyLinks(post.details())
  var replyTo = Email.postReplyAddress(post.id, reader.id)
 
  var groupIds = Activity.groupIds(this.relations.activity)

api/models/Post.js

@@ -1,13 +1,12 @@
 /* globals _ */
-/* eslint-disable camelcase */
 import { difference, filter, isNull, omitBy, uniqBy, isEmpty, intersection, isUndefined, pick } from 'lodash/fp'
-import { compact, flatten, some, sortBy, uniq } from 'lodash'
+import { flatten, sortBy } from 'lodash'
+import { TextHelpers } from 'hylo-shared'
 import { postRoom, pushToSockets } from '../services/Websockets'
 import { fulfill, unfulfill } from './post/fulfillPost'
 import EnsureLoad from './mixins/EnsureLoad'
 import { countTotal } from '../../lib/util/knex'
 import { refineMany, refineOne } from './util/relations'
-import html2text from '../../lib/htmlparser/html2text'
 import ProjectMixin from './project/mixin'
 import EventMixin from './event/mixin'
 
@@ -42,6 +41,44 @@ module.exports = bookshelf.Model.extend(Object.assign({
 
  // Instance Methods
 
+ // Simple attribute getters
+
+ details: function () {
+ // This should be always used when accessing this attribute
+ return TextHelpers.sanitizeHTML(this.get('description'))
+ },
+
+ description: function () {
+ console.warn('Deprecation warning: Post#description called but has been replaced by Post#details')
+ return this.details()
+ },
+
+ title: function () {
+ return this.get('name')
+ },
+
+ isPublic: function () {
+ return this.get('is_public')
+ },
+
+ isWelcome: function () {
+ return this.get('type') === Post.Type.WELCOME
+ },
+
+ isThread: function () {
+ return this.get('type') === Post.Type.THREAD
+ },
+
+ commentsTotal: function () {
+ return this.get('num_comments')
+ },
+
+ votesTotal: function () {
+ return this.get('num_votes')
+ },
+
+ // Relations
+
  activities: function () {
  return this.hasMany(Activity)
  },
@@ -165,10 +202,6 @@ module.exports = bookshelf.Model.extend(Object.assign({
  })
  },
 
- getDetailsText: async function () {
- return html2text(this.get('description'))
- },
-
  // Emulate the graphql request for a post in the feed so the feed can be
  // updated via socket. Some fields omitted, linkPreview for example.
  // TODO: if we were in a position to avoid duplicating the graphql layer
@@ -179,6 +212,7 @@ module.exports = bookshelf.Model.extend(Object.assign({
  const creator = refineOne(user, [ 'id', 'name', 'avatar_url' ])
  const topics = refineMany(tags, [ 'id', 'name' ])
 
+ // TODO: Sanitization -- sanitize details here if not passing through `text` getter
  return Object.assign({},
  refineOne(
  this,
@@ -201,18 +235,6 @@ module.exports = bookshelf.Model.extend(Object.assign({
  )
  },
 
- isPublic: function () {
- return this.get('is_public')
- },
-
- isWelcome: function () {
- return this.get('type') === Post.Type.WELCOME
- },
-
- isThread: function () {
- return this.get('type') === Post.Type.THREAD
- },
-
  async lastReadAtForUser (userId) {
  const pu = await this.postUsers()
  .query(q => q.where('user_id', userId)).fetchOne()
@@ -310,7 +332,7 @@ module.exports = bookshelf.Model.extend(Object.assign({
  reason: `tag: ${tagFollow.relations.tag.get('name')}`
  }))
 
- const mentions = RichText.getUserMentions(this.get('description'))
+ const mentions = RichText.getUserMentions(this.details())
  const mentioned = mentions.map(userId => ({
  reader_id: userId,
  post_id: this.id,

api/models/PushNotification.js

@@ -1,5 +1,5 @@
 import decode from 'ent/decode'
-import truncate from 'trunc-html'
+import { TextHelpers } from 'hylo-shared'
 
 module.exports = bookshelf.Model.extend({
  tableName: 'push_notifications',
@@ -53,7 +53,7 @@ module.exports = bookshelf.Model.extend({
  if (media && media.length !== 0) {
  return `${person} sent an image`
  }
- const blurb = decode(truncate(comment.get('text'), 140).text).trim()
+ const blurb = TextHelpers.presentHTMLToText(comment.text(), { truncate: 140 })
  const postName = comment.relations.post.get('name')
 
  return version === 'mention'

api/models/Tag.js

@@ -5,7 +5,7 @@ import { includes, isUndefined } from 'lodash'
 import {
  filter, omitBy, some, uniq
 } from 'lodash/fp'
-import { validateTopicName } from 'hylo-utils/validators'
+import { Validators } from 'hylo-shared'
 
 export const tagsInText = (text = '') => {
  const re = /(?:^| |>)#([A-Za-z][\w_-]+)/g
@@ -123,11 +123,11 @@ module.exports = bookshelf.Model.extend({
  TagFollow.create({group_id, tag_id, user_id}, opts))),
 
  isValidTag: function (text) {
- return !validateTopicName(text)
+ return !Validators.validateTopicName(text)
  },
 
  validate: function (text) {
- return validateTopicName(text)
+ return Validators.validateTopicName(text)
  },
 
  tagsInText,