Skip to content

Commit 17cf2df

Browse files
committed
fix(release): keep bare secretKey redacted, leave the proxy every-address rule to the transport, and report unknown GitLab hosts as input errors
1 parent 397444b commit 17cf2df

8 files changed

Lines changed: 30 additions & 38 deletions

File tree

‎apps/sim/lib/core/security/redaction.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ describe('isSensitiveKey', () => {
3131
'exa_api_key',
3232
'secretAccessKey',
3333
'awsSecretAccessKey',
34+
'secretKey',
3435
'bedrockSecretKey',
3536
'apiSecretKey',
3637
'clerkSecretKey',
@@ -45,7 +46,6 @@ describe('isSensitiveKey', () => {
4546
it.concurrent.each([
4647
'tokenCount',
4748
'tokenizer',
48-
'secretKey',
4949
'deployKeyName',
5050
'passwordStrength',
5151
'authMethod',

‎apps/sim/lib/core/security/redaction.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,10 @@ const SENSITIVE_KEY_PATTERNS: RegExp[] = [
2929
/^.*api[_-]?key$/i,
3030
// AWS's `secretAccessKey` / `awsSecretAccessKey`, which no suffix above covers.
3131
/^.*secret[_-]?access[_-]?key$/i,
32-
// Prefixed secret keys such as Bedrock's `bedrockSecretKey` and Clerk's `clerkSecretKey`.
33-
// The prefix is required: Infisical returns each secret's name in a bare `secretKey`
34-
// field, and redacting it would hide every listed name.
35-
/^.+secret[_-]?key$/i,
32+
// Secret keys, bare or prefixed: Clerk and Amplitude take their credential in a bare
33+
// `secretKey`, Bedrock in `bedrockSecretKey`. This also masks the secret names Infisical
34+
// returns under `secretKey` in display copies; a hidden name is safer than a leaked key.
35+
/^.*secret[_-]?key$/i,
3636
// Convex's `deployKey`.
3737
/^.*deploy[_-]?key$/i,
3838
/^passphrase$/i,

‎apps/sim/lib/core/utils/route-cache.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ export async function clearRouteCacheFromOtherBuild(nextDirectory: string): Prom
2828
await writeFile(join(routeCache, BUILD_MARKER), buildId)
2929
} catch (error) {
3030
if ((error as NodeJS.ErrnoException).code === 'ENOENT') return
31-
logger.warn('Could not clear the route cache from an earlier build', {
31+
logger.error('Could not clear the route cache from an earlier build', {
3232
error: getErrorMessage(error),
3333
})
3434
}

‎apps/sim/lib/credentials/gitlab-personal-token.test.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,19 @@ describe('GitLab personal token verification', () => {
106106
message: expect.stringContaining("can't connect to that address"),
107107
})
108108
})
109+
it('reports a GitLab host that does not resolve as the user input', async () => {
110+
mocks.fetch.mockReset().mockRejectedValue(
111+
Object.assign(new Error('getaddrinfo ENOTFOUND gitlab.example.invalid'), {
112+
code: 'ENOTFOUND',
113+
})
114+
)
115+
await expect(
116+
verifyGitLabPersonalToken('secret', 'gitlab.example.invalid')
117+
).rejects.toMatchObject({
118+
code: 'validation',
119+
message: expect.stringContaining("couldn't find that GitLab host"),
120+
})
121+
})
109122
it('binds organization tokens independently of the execution workspace', async () => {
110123
mocks.encrypt.mockResolvedValue({ encrypted: 'ciphertext' })
111124
const { workspaceId: _workspaceId, ...identity } = envelope

‎apps/sim/lib/credentials/gitlab-personal-token.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { OrchestrationError } from '@/lib/core/orchestration/types'
44
import { resourceScopeFromOwner, sameResourceScope } from '@/lib/core/resource-scope'
55
import { decryptSecret, encryptSecret } from '@/lib/core/security/encryption'
66
import { secureFetchWithValidation } from '@/lib/core/security/input-validation.server'
7+
import { isHostNotFound } from '@/lib/credentials/token-service-accounts/errors'
78
import { normalizeGitLabHost } from '@/tools/gitlab/utils'
89

910
const gitLabUserSchema = z.object({
@@ -63,6 +64,12 @@ export async function verifyGitLabPersonalToken(accessToken: string, rawHost?: s
6364
"Sim can't connect to that address. Use your GitLab instance's public https URL."
6465
)
6566
}
67+
if (isHostNotFound(error)) {
68+
throw new OrchestrationError(
69+
'validation',
70+
"Sim couldn't find that GitLab host. Check the address and try again."
71+
)
72+
}
6673
throw error
6774
})
6875
if (!response.ok) {

‎apps/sim/lib/credentials/token-service-accounts/validators/gitlab.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ describe('GitLab saved token verification', () => {
7777
validateUrl.mockResolvedValue({
7878
isValid: false,
7979
error: 'url hostname could not be resolved',
80+
refusal: 'dns',
8081
cause: Object.assign(new Error('getaddrinfo ENOTFOUND'), { code: 'ENOTFOUND' }),
8182
})
8283

‎apps/sim/lib/credentials/token-service-accounts/validators/http.test.ts‎

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,9 @@ import {
55
} from '@sim/testing/mocks/input-validation.mock'
66
import { beforeEach, describe, expect, it, vi } from 'vitest'
77

8-
const { mockResolveHost } = vi.hoisted(() => ({ mockResolveHost: vi.fn() }))
9-
108
vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock)
11-
vi.mock('@sim/security/dns', async (importOriginal) => ({
12-
...(await importOriginal<typeof import('@sim/security/dns')>()),
13-
resolveHostAddresses: mockResolveHost,
14-
}))
159

1610
import { EgressRefusedError } from '@/lib/core/network/transport.server'
17-
import type * as InputValidation from '@/lib/core/security/input-validation.server'
1811
import {
1912
normalizeHttpBaseUrl,
2013
refuseUnsafeTypedUrl,
@@ -217,24 +210,6 @@ describe('tokenCheckValidator with a typed host', () => {
217210
})
218211

219212
describe('behind an environment proxy', () => {
220-
it('refuses a host with a private record alongside its public one before sending the token', async () => {
221-
vi.stubEnv('HTTPS_PROXY', 'http://proxy.internal:3128')
222-
const actual = await vi.importActual<typeof InputValidation>(
223-
'@/lib/core/security/input-validation.server'
224-
)
225-
validateUrl.mockImplementation(actual.validateUrlWithDNS)
226-
mockResolveHost.mockResolvedValue({
227-
addresses: ['93.184.216.34', '10.0.0.5'],
228-
preferred: '93.184.216.34',
229-
})
230-
pinnedFetch.mockResolvedValue(jsonResponse({ ok: true }))
231-
232-
await expect(
233-
validateHost({ apiToken: 'good', domain: 'split.example.com' })
234-
).rejects.toMatchObject({ code: 'host_not_allowed', status: 400 })
235-
expect(pinnedFetch).not.toHaveBeenCalled()
236-
})
237-
238213
it("reports the proxy route's own refusal as the host, not an outage", async () => {
239214
pinnedFetch.mockRejectedValue(new EgressRefusedError('url resolves to a private address'))
240215

‎apps/sim/lib/credentials/token-service-accounts/validators/http.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { resolveEnvironmentProxy } from '@sim/security/environment-proxy'
21
import { truncate } from '@sim/utils/string'
32
import { EgressRefusedError } from '@/lib/core/network/transport.server'
43
import type { EgressProfile } from '@/lib/core/security/egress/profiles'
@@ -183,12 +182,9 @@ async function sendThroughEgress(
183182
profile: NonNullable<TokenCheck['egress']>,
184183
{ url, method = 'GET', headers, body }: TokenCheckRequest
185184
): Promise<ProviderResponse> {
186-
// A forward proxy re-resolves the host, so the transport refuses one with any refused record.
187-
// An unparseable URL skips the proxy lookup; validation then refuses it as the user's input.
188-
const validation = await validateUrlWithDNS(url, 'url', profile, {
189-
logDetails: false,
190-
requireEveryAddress: URL.canParse(url) && resolveEnvironmentProxy(new URL(url)) !== null,
191-
})
185+
// The transport applies the stricter every-address rule itself when it routes through an
186+
// environment proxy; its refusal is classified below, so a gateway route is not pre-judged.
187+
const validation = await validateUrlWithDNS(url, 'url', profile, { logDetails: false })
192188
if (!validation.isValid) {
193189
if (validation.refusal !== 'dns') throw hostNotAllowed()
194190
const notFound = isHostNotFound(validation.cause)

0 commit comments

Comments
 (0)