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
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "latchkey",
"version": "2.15.3",
"version": "2.15.4",
"description": "A CLI tool that injects API credentials into curl requests to third-party services",
"author": "Imbue <hynek@imbue.com>",
"repository": {
Expand Down
20 changes: 19 additions & 1 deletion src/curlInjection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,16 @@ export interface CurlInjectionDependencies {
export async function prepareCurlInvocation(
curlArguments: readonly string[],
apiCredentialStore: ApiCredentialStore,
dependencies: CurlInjectionDependencies
dependencies: CurlInjectionDependencies,
/**
* The actual request body, when the caller has it available in memory but
* passes it to curl out-of-band (e.g. the gateway streams it via
* `--data-binary @-` on stdin). In that case the parsed curl arguments only
* contain the placeholder `@-`, so the permission check would otherwise see
* `"@-"` as the body. Supplying it here lets the permission check inspect
* the real body without changing how curl is actually invoked.
*/
requestBodyForPermissionCheck?: Buffer | null
): Promise<readonly string[]> {
// Parse the curl arguments once for the permission check. A parse failure
// here means the user's curl invocation is malformed, which is treated as
Expand All @@ -103,6 +112,15 @@ export async function prepareCurlInvocation(
}
throw error;
}
// When the real body was supplied out-of-band, rebuild the request so the
// permission check sees the actual payload instead of the `@-` placeholder.
if (requestBodyForPermissionCheck !== undefined && requestBodyForPermissionCheck !== null) {
parsedRequest = new Request(parsedRequest.url, {
method: parsedRequest.method,
headers: parsedRequest.headers,
body: requestBodyForPermissionCheck,
});
}
const allowed = await dependencies.checkPermission(
parsedRequest,
dependencies.permissionsConfigPath,
Expand Down
25 changes: 17 additions & 8 deletions src/gateway/gatewayEndpoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -335,14 +335,23 @@ export async function handleGatewayRequest(

let allArguments: readonly string[];
try {
allArguments = await prepareCurlInvocation(curlArguments, apiCredentialStore, {
registry: deps.registry,
checkPermission: deps.checkPermission,
permissionsConfigPath,
permissionsDoNotUseBuiltinSchemas: deps.config.permissionsDoNotUseBuiltinSchemas,
passthroughUnknown: deps.config.passthroughUnknown,
credentialsRefreshDisabled: deps.config.credentialsRefreshDisabled,
});
allArguments = await prepareCurlInvocation(
curlArguments,
apiCredentialStore,
{
registry: deps.registry,
checkPermission: deps.checkPermission,
permissionsConfigPath,
permissionsDoNotUseBuiltinSchemas: deps.config.permissionsDoNotUseBuiltinSchemas,
passthroughUnknown: deps.config.passthroughUnknown,
credentialsRefreshDisabled: deps.config.credentialsRefreshDisabled,
},
// The gateway forwards the body to curl out-of-band via
// `--data-binary @-` on stdin, so the parsed curl arguments only carry
// the `@-` placeholder. Hand the real body to the permission check so it
// inspects the actual payload.
body
);
} catch (error) {
if (error instanceof RequestNotPermittedError) {
deps.log(`${method} ${targetUrl} -> 403`);
Expand Down
2 changes: 1 addition & 1 deletion src/version.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
// Auto-generated from package.json by scripts/generateVersion.js.
// Do not edit by hand; run `node scripts/generateVersion.js` to refresh.
export const VERSION = "2.15.3";
export const VERSION = "2.15.4";
23 changes: 22 additions & 1 deletion tests/gateway.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ describe('gateway server', () => {
let errorLogs: string[];
let capturedCurlArgs: readonly string[];
let capturedCurlStdin: Buffer | undefined;
let capturedPermissionCheckBody: string | undefined;
let mockCurlResponse: AsyncCurlResult;
let mockCurlHeaderDump: string;
let mockPermissionResult: boolean;
Expand Down Expand Up @@ -260,7 +261,10 @@ describe('gateway server', () => {

return mockCurlResponse;
},
checkPermission: () => Promise.resolve(mockPermissionResult),
checkPermission: async (request: Request): Promise<boolean> => {
capturedPermissionCheckBody = await request.clone().text();
return mockPermissionResult;
},
confirm: () => Promise.resolve(true),
exit: (code: number): never => {
throw new Error(`process.exit(${String(code)})`);
Expand Down Expand Up @@ -310,6 +314,7 @@ describe('gateway server', () => {
errorLogs = [];
capturedCurlArgs = [];
capturedCurlStdin = undefined;
capturedPermissionCheckBody = undefined;
mockPermissionResult = true;
mockCurlResponse = {
returncode: 0,
Expand Down Expand Up @@ -383,6 +388,22 @@ describe('gateway server', () => {
expect(capturedCurlStdin?.toString()).toBe(requestBody);
});

it('should expose the real body (not the @- placeholder) to the permission check', async () => {
gateway = await createTestGateway();
const requestBody = '{"channel":"C01","text":"hello"}';

await fetch('/gateway/https://slack.com/api/chat.postMessage', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: requestBody,
});

// The body is streamed to curl via `--data-binary @-`, but the
// permission check must still see the actual payload.
expect(capturedPermissionCheckBody).toBe(requestBody);
expect(capturedPermissionCheckBody).not.toBe('@-');
});

it('should forward query parameters in the target URL', async () => {
gateway = await createTestGateway();

Expand Down
Loading