Skip to content

Commit a5badf0

Browse files
authored
fix(codeql): address validation findings (#7520)
1 parent 788669a commit a5badf0

4 files changed

Lines changed: 95 additions & 6 deletions

File tree

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/table-serialization.node.test.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,4 +97,54 @@ describe('legacy table preservation through the server converter', () => {
9797
shared.destroy()
9898
}
9999
})
100+
101+
it('uses lossless HTML when a code span has a backslash immediately before a pipe', () => {
102+
const document: JSONContent = {
103+
type: 'doc',
104+
content: [
105+
{
106+
type: 'table',
107+
content: [
108+
{
109+
type: 'tableRow',
110+
content: [
111+
{
112+
type: 'tableHeader',
113+
content: [{ type: 'paragraph', content: [{ type: 'text', text: 'value' }] }],
114+
},
115+
],
116+
},
117+
{
118+
type: 'tableRow',
119+
content: [
120+
{
121+
type: 'tableCell',
122+
content: [
123+
{
124+
type: 'paragraph',
125+
content: [{ type: 'text', text: '\\|', marks: [{ type: 'code' }] }],
126+
},
127+
],
128+
},
129+
],
130+
},
131+
],
132+
},
133+
],
134+
}
135+
const shared = prosemirrorJSONToYDoc(
136+
getSchema(createMarkdownContentExtensions()),
137+
document,
138+
COLLAB_DOC_FIELD
139+
)
140+
try {
141+
const markdown = yDocToMarkdown(shared).trim()
142+
expect(markdown).toContain('<table')
143+
expect(markdown).toContain('<code>\\|</code>')
144+
expect(parseMarkdownToDoc(markdown).content?.[0].type).toBe('rawHtmlBlock')
145+
expect(serializeMarkdownDocument(markdown).trim()).toBe(markdown)
146+
} finally {
147+
shared.destroy()
148+
}
149+
})
100150
})

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/table.ts

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,30 @@ const MarkdownTable = Table.extend({
166166
},
167167
})
168168

169+
/** Whether a code span contains a pipe after an odd run of literal backslashes. */
170+
function hasAmbiguousCodePipe(value: string): boolean {
171+
let backslashes = 0
172+
for (const character of value) {
173+
if (character === '\\') {
174+
backslashes++
175+
continue
176+
}
177+
if (character === '|' && backslashes % 2 === 1) return true
178+
backslashes = 0
179+
}
180+
return false
181+
}
182+
183+
/** Whether inline cell content can be represented without ambiguity in a GFM pipe table. */
184+
function isGfmCellContent(content: JSONContent[] | undefined): boolean {
185+
return !(content ?? []).some(
186+
(node) =>
187+
node.type === 'text' &&
188+
node.marks?.some((mark) => mark.type === 'code') &&
189+
hasAmbiguousCodePipe(node.text ?? '')
190+
)
191+
}
192+
169193
/** A GFM table has a single header, uniform columns, inline cell content, and column-level alignment. */
170194
function isGfmTable(node: JSONContent): boolean {
171195
const rows = node.content ?? []
@@ -183,12 +207,20 @@ function isGfmTable(node: JSONContent): boolean {
183207
cell.attrs?.colwidth == null &&
184208
(cell.attrs?.align ?? null) === (header[column].attrs?.align ?? null) &&
185209
cell.content?.length === 1 &&
186-
cell.content[0].type === 'paragraph'
210+
cell.content[0].type === 'paragraph' &&
211+
isGfmCellContent(cell.content[0].content)
187212
)
188213
)
189214
)
190215
}
191216

217+
/** Escape pipes that belong to cell content while leaving the structural delimiters to the renderer. */
218+
function escapeGfmCellPipes(value: string): string {
219+
const escaped: string[] = []
220+
for (const character of value) escaped.push(character === '|' ? '\\|' : character)
221+
return escaped.join('')
222+
}
223+
192224
/**
193225
* Render compatible cells without collapsing interior whitespace: spaces inside code spans are
194226
* content, not table padding. Formatting padding is bounded so one wide cell cannot multiply its
@@ -197,10 +229,7 @@ function isGfmTable(node: JSONContent): boolean {
197229
function renderGfmTable(node: JSONContent, helpers: MarkdownRendererHelpers): string {
198230
const rows = (node.content ?? []).map((row) =>
199231
(row.content ?? []).map((cell) =>
200-
helpers
201-
.renderChildren(cell.content ?? [])
202-
.replace(/\|/g, '\\|')
203-
.replace(/\r?\n/g, '<br>')
232+
escapeGfmCellPipes(helpers.renderChildren(cell.content ?? [])).replace(/\r?\n/g, '<br>')
204233
)
205234
)
206235
const widths = rows[0].map(() => 3)

apps/sim/lib/api/contracts/tools/aws/cloudtrail-shared.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ const TRAIL_NAME_MAX_LENGTH = 128
1313
const TRAIL_ARN_MAX_LENGTH = 256
1414

1515
const TRAIL_NAME_OR_ARN_PATTERN =
16-
/^(?:arn:aws[a-zA-Z0-9-]*:cloudtrail:[a-z0-9-]+:\d{12}:trail\/[\w.\-/]+|[a-zA-Z0-9](?:[._-]?[a-zA-Z0-9]+)+)$/
16+
/^(?:arn:aws[a-zA-Z0-9-]*:cloudtrail:[a-z0-9-]+:\d{12}:trail\/[\w.\-/]+|[a-zA-Z0-9](?:[a-zA-Z0-9]|[._-][a-zA-Z0-9])+)$/
1717

1818
/**
1919
* A trail name or a full trail ARN. The two branches carry different ceilings: a bare name

apps/sim/tools/cloudtrail/contract-validation.test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ const CONNECTION = {
2828
const QUERY_ID = 'abcdef01-2345-6789-abcd-ef0123456789'
2929
const LONGEST_VALID_NAME = 'a'.repeat(128)
3030
const TOO_LONG_NAME = 'a'.repeat(129)
31+
const LONG_INVALID_NAME = `${'a'.repeat(128)}!`
3132
const LONG_TRAIL_ARN = `arn:aws:cloudtrail:us-east-1:123456789012:trail/${'a'.repeat(100)}`
3233

3334
describe('cloudtrail start query contract', () => {
@@ -102,6 +103,15 @@ describe('cloudtrail trail name bounds', () => {
102103
expect(result.success).toBe(true)
103104
})
104105

106+
it('rejects a long malformed trail name', () => {
107+
const result = awsCloudtrailGetTrailStatusContract.body.safeParse({
108+
...CONNECTION,
109+
name: LONG_INVALID_NAME,
110+
})
111+
112+
expect(result.success).toBe(false)
113+
})
114+
105115
it('accepts a trail ARN longer than 128 characters on get trail status', () => {
106116
const result = awsCloudtrailGetTrailStatusContract.body.safeParse({
107117
...CONNECTION,

0 commit comments

Comments
 (0)