Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ Below are some example URLs that generate the tokens we see most often:
* [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 -->
* [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)
* [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 -->
* [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 -->
* [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 -->

#### Supported Query Parameters

Expand Down
2 changes: 1 addition & 1 deletion content/copilot/reference/customization-cheat-sheet.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ This table shows which customization features are supported in each IDE and surf
* ✗ = not supported
* P = under preview

| 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 %} |
| 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 %} |
|---------|:-------:|:-------------:|:---------:|:-------:|:-----:|:-------:|:---:|
| Custom instructions | ✓ | ✓ | P | P | P | ✓ | ✓ |
| Prompt files | ✓ | ✓ | P | ✗ | P | ✗ | ✓ |
Expand Down
3 changes: 1 addition & 2 deletions src/shielding/middleware/handle-invalid-paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ export default function handleInvalidPaths(
// We can all the CDN to cache these responses because they're
// they're not going to suddenly work in the next deployment.
defaultCacheControl(res)
res.setHeader('content-type', 'text/plain')
res.status(404).send('Not found')
res.status(404).type('text').send('Not found')
return
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ export default function handleInvalidQuerystringValues(

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

const tags = ['response:400', `path:${req.path}`, `key:${key}`]
statsd.increment(STATSD_KEY, 1, tags)
Expand Down
5 changes: 2 additions & 3 deletions src/shielding/middleware/handle-invalid-query-strings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,7 @@ export default function handleInvalidQuerystrings(

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

const tags = [
'response:400',
Expand Down Expand Up @@ -105,7 +104,7 @@ export default function handleInvalidQuerystrings(
noCacheControl(res)

const message = honeypotted ? 'Honeypotted' : 'Too many unrecognized query string parameters'
res.status(400).send(message)
res.status(400).type('text').send(message)

const tags = [
'response:400',
Expand Down
3 changes: 1 addition & 2 deletions src/shielding/middleware/handle-malformed-urls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ export default function handleMalformedUrls(
} catch {
// If any decoding fails, this is a malformed URL
defaultCacheControl(res)
res.setHeader('content-type', 'text/plain')
res.status(400).send('Bad Request: Malformed URL')
res.status(400).type('text').send('Bad Request: Malformed URL')
return
}

Expand Down
19 changes: 17 additions & 2 deletions src/shielding/tests/invalid-querystrings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ describe('invalid query strings', () => {
const url = `/?${sp}`
const res = await get(url)
expect(res.statusCode).toBe(400)
expect(res.headers['content-type']).toMatch('text/plain')
expect(res.headers['cache-control']).toMatch('no-store')
expect(res.headers['cache-control']).toMatch('private')
})
Expand Down Expand Up @@ -69,14 +70,20 @@ describe('invalid query strings', () => {
const url = `/en?query[foo]=bar`
const res = await get(url)
expect(res.statusCode).toBe(400)
expect(res.body).toMatch('Invalid query string key (query)')
expect(res.headers['content-type']).toMatch('text/plain')
expect(res.body).toMatch('Invalid query string')
// Must not reflect the user-supplied key name
expect(res.body).not.toContain('(query)')
})

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

test('bad tool query string with Chinese URL-encoded characters', async () => {
Expand All @@ -86,6 +93,14 @@ describe('invalid query strings', () => {
expect(res.statusCode).toBe(302)
expect(res.headers.location).toBe('/?tool=azure_data_studio')
})

test('XSS payloads in bracket query keys are not reflected', async () => {
const res = await get('/en?%3Cscript%3Ealert()%3C/script%3E[]')
expect(res.statusCode).toBe(400)
expect(res.headers['content-type']).toMatch('text/plain')
expect(res.body).not.toContain('<script>')
expect(res.body).not.toContain('alert')
})
})

function randomCharacters(length: number) {
Expand Down
59 changes: 59 additions & 0 deletions src/shielding/tests/middleware-security.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import fs from 'fs'
import path from 'path'

import { describe, expect, test } from 'vitest'

const middlewareDir = path.join(__dirname, '..', 'middleware')

describe('shielding middleware security patterns', () => {
const middlewareFiles = fs.readdirSync(middlewareDir).filter((f) => f.endsWith('.ts'))

test("every .send() call uses .type('text')", () => {
const violations: string[] = []

for (const file of middlewareFiles) {
// index.ts is just the router, skip it
if (file === 'index.ts') continue

const content = fs.readFileSync(path.join(middlewareDir, file), 'utf-8')
const lines = content.split('\n')

for (let i = 0; i < lines.length; i++) {
const line = lines[i]
if (line.includes('.send(') && !line.includes(".type('text')")) {
violations.push(`${file}:${i + 1}: ${line.trim()}`)
}
}
}

expect(
violations,
`All .send() calls in shielding middleware must use .type('text') to prevent XSS.\n` +
`Violations:\n${violations.join('\n')}`,
).toHaveLength(0)
})

test('no .send() call reflects user input via template literals', () => {
const violations: string[] = []

for (const file of middlewareFiles) {
if (file === 'index.ts') continue

const content = fs.readFileSync(path.join(middlewareDir, file), 'utf-8')
const lines = content.split('\n')

for (let i = 0; i < lines.length; i++) {
const line = lines[i]
if (line.includes('.send(') && line.includes('${')) {
violations.push(`${file}:${i + 1}: ${line.trim()}`)
}
}
}

expect(
violations,
`Shielding middleware must not reflect user input in .send() responses.\n` +
`Violations:\n${violations.join('\n')}`,
).toHaveLength(0)
})
})
1 change: 1 addition & 0 deletions src/shielding/tests/shielding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ describe('honeypotting', () => {
test('any GET with survey-vote and survey-token query strings is 400', async () => {
const res = await get('/en?survey-vote=1&survey-token=2')
expect(res.statusCode).toBe(400)
expect(res.headers['content-type']).toMatch('text/plain')
expect(res.body).toMatch(/Honeypotted/)
expect(res.headers['cache-control']).toMatch('private')
})
Expand Down