Skip to content

Commit cb4fcef

Browse files
authored
Merge pull request #43196 from github/repo-sync
Repo sync
2 parents 3d1f989 + efed051 commit cb4fcef

File tree

9 files changed

+85
-13
lines changed

9 files changed

+85
-13
lines changed

content/authentication/keeping-your-account-and-data-secure/managing-your-personal-access-tokens.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ Below are some example URLs that generate the tokens we see most often:
144144
* [GitHub Models access](https://github.com/settings/personal-access-tokens/new?name=GitHub+Models+token&description=Used%20to%20call%20GitHub%20Models%20APIs%20to%20easily%20run%20LLMs%3A%20https%3A%2F%2Fdocs.github.com%2Fgithub-models%2Fquickstart%23step-2-make-an-api-call&user_models=read)<!-- markdownlint-disable-line search-replace Custom rule -->
145145
* [Update code and open a PR](https://github.com/settings/personal-access-tokens/new?name=Core-loop+token&description=Write%20code%20and%20push%20it%20to%20main%21%20Includes%20permission%20to%20edit%20workflow%20files%20for%20Actions%20-%20remove%20%60workflows%3Awrite%60%20if%20you%20don%27t%20need%20to%20do%20that&contents=write&pull_requests=write&workflows=write)
146146
* [Manage Copilot licenses in an organization](https://github.com/settings/personal-access-tokens/new?name=Core-loop+token&description=Enable%20or%20disable%20copilot%20access%20for%20users%20with%20the%20Seat%20Management%20APIs%3A%20https%3A%2F%2Fdocs.github.com%2Frest%2Fcopilot%2Fcopilot-user-management%0ABe%20sure%20to%20select%20an%20organization%20for%20your%20resource%20owner%20below%21&organization_copilot_seat_management=write)<!-- markdownlint-disable-line search-replace Custom rule -->
147-
* [Make Copilot requests](https://github.com/settings/personal-access-tokens/new?name=Copilot+requests+token&description=Make%20Copilot%20API%20requests%20on%20behalf%20of%20the%20user%2C%20consuming%20premium%20requests%3A%20https%3A%2F%2Fdocs.github.com%2Fcopilot%2Fconcepts%2Fbilling%2Fcopilot-requests&copilot_requests=write)<!-- markdownlint-disable-line search-replace Custom rule -->
147+
* [Make Copilot requests](https://github.com/settings/personal-access-tokens/new?name=Copilot+requests+token&description=Make%20Copilot%20API%20requests%20on%20behalf%20of%20the%20user%2C%20consuming%20premium%20requests%3A%20https%3A%2F%2Fdocs.github.com%2Fcopilot%2Fconcepts%2Fbilling%2Fcopilot-requests&user_copilot_requests=read)<!-- markdownlint-disable-line search-replace Custom rule -->
148148

149149
#### Supported Query Parameters
150150

content/copilot/reference/customization-cheat-sheet.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ This table shows which customization features are supported in each IDE and surf
4949
* ✗ = not supported
5050
* P = under preview
5151

52-
| Feature | {% data variables.product.prodname_vscode_shortname %} | {% data variables.product.prodname_vs %} | JetBrains IDEs | Eclipse | Xcode | <span style="white-space: nowrap;">{% data variables.product.prodname_dotcom_the_website %}</span> | {% data variables.copilot.copilot_cli_short %} |
52+
| Feature | {% data variables.product.prodname_vscode_shortname %} | {% data variables.product.prodname_vs %} | JetBrains IDEs | Eclipse | Xcode | {% data variables.product.prodname_dotcom %} .com | {% data variables.copilot.copilot_cli_short %} |
5353
|---------|:-------:|:-------------:|:---------:|:-------:|:-----:|:-------:|:---:|
5454
| Custom instructions ||| P | P | P |||
5555
| Prompt files ||| P || P |||

src/shielding/middleware/handle-invalid-paths.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,7 @@ export default function handleInvalidPaths(
7979
// We can all the CDN to cache these responses because they're
8080
// they're not going to suddenly work in the next deployment.
8181
defaultCacheControl(res)
82-
res.setHeader('content-type', 'text/plain')
83-
res.status(404).send('Not found')
82+
res.status(404).type('text').send('Not found')
8483
return
8584
}
8685

src/shielding/middleware/handle-invalid-query-string-values.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,9 @@ export default function handleInvalidQuerystringValues(
7171

7272
// For example ?foo[bar]=baz (but not ?foo=bar&foo=baz)
7373
if (value instanceof Object && !Array.isArray(value)) {
74-
const message = `Invalid query string key (${key})`
74+
const message = 'Invalid query string'
7575
defaultCacheControl(res)
76-
res.status(400).send(message)
76+
res.status(400).type('text').send(message)
7777

7878
const tags = ['response:400', `path:${req.path}`, `key:${key}`]
7979
statsd.increment(STATSD_KEY, 1, tags)

src/shielding/middleware/handle-invalid-query-strings.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,7 @@ export default function handleInvalidQuerystrings(
7070

7171
if (invalidKeys.length > 0) {
7272
noCacheControl(res)
73-
const invalidKey = invalidKeys[0].replace(/\[.*$/, '') // Get the base key name
74-
res.status(400).send(`Invalid query string key (${invalidKey})`)
73+
res.status(400).type('text').send('Invalid query string')
7574

7675
const tags = [
7776
'response:400',
@@ -105,7 +104,7 @@ export default function handleInvalidQuerystrings(
105104
noCacheControl(res)
106105

107106
const message = honeypotted ? 'Honeypotted' : 'Too many unrecognized query string parameters'
108-
res.status(400).send(message)
107+
res.status(400).type('text').send(message)
109108

110109
const tags = [
111110
'response:400',

src/shielding/middleware/handle-malformed-urls.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,7 @@ export default function handleMalformedUrls(
2222
} catch {
2323
// If any decoding fails, this is a malformed URL
2424
defaultCacheControl(res)
25-
res.setHeader('content-type', 'text/plain')
26-
res.status(400).send('Bad Request: Malformed URL')
25+
res.status(400).type('text').send('Bad Request: Malformed URL')
2726
return
2827
}
2928

src/shielding/tests/invalid-querystrings.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ describe('invalid query strings', () => {
2121
const url = `/?${sp}`
2222
const res = await get(url)
2323
expect(res.statusCode).toBe(400)
24+
expect(res.headers['content-type']).toMatch('text/plain')
2425
expect(res.headers['cache-control']).toMatch('no-store')
2526
expect(res.headers['cache-control']).toMatch('private')
2627
})
@@ -69,14 +70,20 @@ describe('invalid query strings', () => {
6970
const url = `/en?query[foo]=bar`
7071
const res = await get(url)
7172
expect(res.statusCode).toBe(400)
72-
expect(res.body).toMatch('Invalid query string key (query)')
73+
expect(res.headers['content-type']).toMatch('text/plain')
74+
expect(res.body).toMatch('Invalid query string')
75+
// Must not reflect the user-supplied key name
76+
expect(res.body).not.toContain('(query)')
7377
})
7478

7579
test('query string keys with square brackets', async () => {
7680
const url = `/?constructor[foo][bar]=buz`
7781
const res = await get(url)
7882
expect(res.statusCode).toBe(400)
79-
expect(res.body).toMatch('Invalid query string key (constructor)')
83+
expect(res.headers['content-type']).toMatch('text/plain')
84+
expect(res.body).toMatch('Invalid query string')
85+
// Must not reflect the user-supplied key name
86+
expect(res.body).not.toContain('(constructor)')
8087
})
8188

8289
test('bad tool query string with Chinese URL-encoded characters', async () => {
@@ -86,6 +93,14 @@ describe('invalid query strings', () => {
8693
expect(res.statusCode).toBe(302)
8794
expect(res.headers.location).toBe('/?tool=azure_data_studio')
8895
})
96+
97+
test('XSS payloads in bracket query keys are not reflected', async () => {
98+
const res = await get('/en?%3Cscript%3Ealert()%3C/script%3E[]')
99+
expect(res.statusCode).toBe(400)
100+
expect(res.headers['content-type']).toMatch('text/plain')
101+
expect(res.body).not.toContain('<script>')
102+
expect(res.body).not.toContain('alert')
103+
})
89104
})
90105

91106
function randomCharacters(length: number) {
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import fs from 'fs'
2+
import path from 'path'
3+
4+
import { describe, expect, test } from 'vitest'
5+
6+
const middlewareDir = path.join(__dirname, '..', 'middleware')
7+
8+
describe('shielding middleware security patterns', () => {
9+
const middlewareFiles = fs.readdirSync(middlewareDir).filter((f) => f.endsWith('.ts'))
10+
11+
test("every .send() call uses .type('text')", () => {
12+
const violations: string[] = []
13+
14+
for (const file of middlewareFiles) {
15+
// index.ts is just the router, skip it
16+
if (file === 'index.ts') continue
17+
18+
const content = fs.readFileSync(path.join(middlewareDir, file), 'utf-8')
19+
const lines = content.split('\n')
20+
21+
for (let i = 0; i < lines.length; i++) {
22+
const line = lines[i]
23+
if (line.includes('.send(') && !line.includes(".type('text')")) {
24+
violations.push(`${file}:${i + 1}: ${line.trim()}`)
25+
}
26+
}
27+
}
28+
29+
expect(
30+
violations,
31+
`All .send() calls in shielding middleware must use .type('text') to prevent XSS.\n` +
32+
`Violations:\n${violations.join('\n')}`,
33+
).toHaveLength(0)
34+
})
35+
36+
test('no .send() call reflects user input via template literals', () => {
37+
const violations: string[] = []
38+
39+
for (const file of middlewareFiles) {
40+
if (file === 'index.ts') continue
41+
42+
const content = fs.readFileSync(path.join(middlewareDir, file), 'utf-8')
43+
const lines = content.split('\n')
44+
45+
for (let i = 0; i < lines.length; i++) {
46+
const line = lines[i]
47+
if (line.includes('.send(') && line.includes('${')) {
48+
violations.push(`${file}:${i + 1}: ${line.trim()}`)
49+
}
50+
}
51+
}
52+
53+
expect(
54+
violations,
55+
`Shielding middleware must not reflect user input in .send() responses.\n` +
56+
`Violations:\n${violations.join('\n')}`,
57+
).toHaveLength(0)
58+
})
59+
})

src/shielding/tests/shielding.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ describe('honeypotting', () => {
77
test('any GET with survey-vote and survey-token query strings is 400', async () => {
88
const res = await get('/en?survey-vote=1&survey-token=2')
99
expect(res.statusCode).toBe(400)
10+
expect(res.headers['content-type']).toMatch('text/plain')
1011
expect(res.body).toMatch(/Honeypotted/)
1112
expect(res.headers['cache-control']).toMatch('private')
1213
})

0 commit comments

Comments
 (0)