Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Credscan #871

Closed
wants to merge 3 commits into from
Closed
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
11 changes: 6 additions & 5 deletions packages/core/src/chat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ import {
serializeLogProb,
topLogprobsToMarkdown,
} from "./logprob"
import { hideSecrets } from "./secrets"

export function toChatCompletionUserMessage(
expanded: string,
Expand Down Expand Up @@ -967,11 +968,11 @@ export function appendUserMessage(
) {
if (!content) return
const last = messages.at(-1) as ChatCompletionUserMessageParam
if (last?.role === "user") last.content += "\n" + content
if (last?.role === "user") last.content += "\n" + hideSecrets(content)
else
messages.push({
role: "user",
content,
content: hideSecrets(content),
} as ChatCompletionUserMessageParam)
}

Expand All @@ -981,11 +982,11 @@ export function appendAssistantMessage(
) {
if (!content) return
const last = messages.at(-1) as ChatCompletionAssistantMessageParam
if (last?.role === "assistant") last.content += "\n" + content
if (last?.role === "assistant") last.content += "\n" + hideSecrets(content)
else
messages.push({
role: "assistant",
content,
content: hideSecrets(content),
} satisfies ChatCompletionAssistantMessageParam)
}

Expand All @@ -1003,7 +1004,7 @@ export function appendSystemMessage(
messages.unshift(last)
}
if (last.content) last.content += SYSTEM_FENCE
last.content += content
last.content += hideSecrets(content)
}

export function addToolDefinitionsMessage(
Expand Down
58 changes: 58 additions & 0 deletions packages/core/src/secrets.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { TraceOptions } from "./trace"
import { logVerbose } from "./util"

const secretPatterns: Record<string, RegExp> = {
"AWS Access Key": /AKIA[0-9A-Z]{16}/g,
"GitHub Token": /ghp_[0-9a-zA-Z]{36}/g,
"Slack Token": /xox[baprs]-[0-9a-zA-Z]{10,48}/g,
"Google API Key": /AIza[0-9A-Za-z-_]{35}/g,
"Azure Key": /[0-9a-zA-Z/+]{88}/g,
"Stripe API Key": /sk_live_[0-9a-zA-Z]{24}/g,
"Google AI Key": /AIza[0-9A-Za-z-_]{35}/g,
"OpenAI Key": /sk-[0-9a-zA-Z]{32}/g,
"Twilio API Key": /SK[0-9a-fA-F]{32}/g,
"SendGrid API Key": /SG\.[0-9A-Za-z\-_]{22}\.[0-9A-Za-z\-_]{43}/g,
"Facebook Access Token": /EAACEdEose0cBA[0-9A-Za-z]+/g,
"Twitter Access Token": /[1-9][0-9]+-[0-9a-zA-Z]{40}/g,
"Twitter Secret Key": /[0-9a-zA-Z]{40}/g,
"GitLab Personal Access Token": /glpat-[0-9a-zA-Z\-_]{20}/g,
"DigitalOcean Token": /[0-9a-fA-F]{64}/g,
"Mailgun API Key": /key-[0-9a-zA-Z]{32}/g,
"Dropbox Access Token": /sl.[0-9a-zA-Z\-_]{43}/g,
"Shopify Access Token": /shpat_[0-9a-fA-F]{32}/g,
"GitHub Personal Access Token": /ghp_[0-9a-zA-Z]{36}/g,
"Generic API Key": /(?<![a-zA-Z0-9])[a-zA-Z0-9]{32,128}(?![a-zA-Z0-9])/g,
"JWT Token": /^[A-Za-z0-9-_=]+\.[A-Za-z0-9-_=]+\.[A-Za-z0-9-_.+/=]*$/g,

Check failure on line 25 in packages/core/src/secrets.ts

View workflow job for this annotation

GitHub Actions / build

The regular expression for "JWT Token" is too permissive and may match unintended strings. Consider refining the pattern to ensure it only matches valid JWT tokens.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regular expression for "JWT Token" is too permissive and may match unintended strings. Consider refining the pattern to ensure it only matches valid JWT tokens.

generated by pr-review-commit regex_incorrect

"AWS Access Key ID": /AKIA[0-9A-Z]{16}/g,
"AWS Secret Access Key":
/(?<![A-Za-z0-9/+=])[A-Za-z0-9/+=]{40}(?![A-Za-z0-9/+=])/g,
"Google Cloud API Key": /AIza[0-9A-Za-z-_]{35}/g,
"Azure Shared Key": /[A-Za-z0-9+=]{88}/g,
"Salesforce Security Token": /[A-Za-z0-9]{24}/g,
"Private SSH Key":
/-----BEGIN PRIVATE KEY-----[\s\S]+?-----END PRIVATE KEY-----/g,
"PEM Certificate":
/-----BEGIN CERTIFICATE-----[\s\S]+?-----END CERTIFICATE-----/g,
"PayPal/Braintree Access Token":
/access_token\$production\$[0-9a-z]{16}\$[0-9a-f]{32}/g,
"Database Connection String": /(?:jdbc|odbc):[a-zA-Z0-9@:\/\.\?&=]+/g,
"Basic Auth Header": /Basic [a-zA-Z0-9=+\/]+/g,
"Bearer Token Header": /Bearer [A-Za-z0-9\-_]+/g,
"Google OAuth Refresh Token": /1\/[A-Za-z0-9_-]{43}/g,
"Kubernetes Secret": /apiVersion:\s+v1[\s\S]+kind:\s+Secret[\s\S]+data:/g,
"Firebase Database URL": /https:\/\/[a-z0-9-]+\.firebaseio\.com/g,
}

export function hideSecrets(text: string): string {
if (!text) return text
let result = text
for (const pattern of Object.entries(secretPatterns)) {
let prev = result
result = result.replace(pattern[1], "***")
if (prev !== result) {
logVerbose(`secrets: hidding potential ${pattern[0]} secret`)

Check failure on line 53 in packages/core/src/secrets.ts

View workflow job for this annotation

GitHub Actions / build

There's a typo in the log message: "hidding" should be "hiding". This can lead to confusion when reading logs.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a typo in the log message: "hidding" should be "hiding". This can lead to confusion when reading logs.

generated by pr-review-commit typo

process.stderr.write(pattern[1].exec(prev) + "\n")

Check failure on line 54 in packages/core/src/secrets.ts

View workflow job for this annotation

GitHub Actions / build

Using `process.stderr.write` to log potential secrets can inadvertently expose sensitive information. Consider using a more secure logging mechanism that redacts or anonymizes the data.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using process.stderr.write to log potential secrets can inadvertently expose sensitive information. Consider using a more secure logging mechanism that redacts or anonymizes the data.

generated by pr-review-commit potential_security_risk

}
}
return result
}
5 changes: 3 additions & 2 deletions packages/core/src/trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import prettyBytes from "pretty-bytes"
import { host } from "./host"
import { ellipse, renderWithPrecision, toStringList } from "./util"
import { estimateTokens } from "./tokens"
import { hideSecrets } from "./secrets"

export class TraceChunkEvent extends Event {
constructor(readonly chunk: string) {
Expand Down Expand Up @@ -64,13 +65,13 @@ export class MarkdownTrace extends EventTarget implements ToolCallTrace {
this.dispatchEvent(new Event(TRACE_DETAILS))
)
trace.startDetails(title)
this._content.push(trace)
this._content.push(hideSecrets(title))
return trace
}

appendContent(value: string) {
if (value !== undefined && value !== null && value !== "") {
this._content.push(value)
this._content.push(hideSecrets(value))
this._tree = undefined
this.dispatchChange()
this.dispatchEvent(new TraceChunkEvent(value))
Expand Down