Skip to content

Commit

Permalink
Improve logged Circomspect file paths when running in Docker
Browse files Browse the repository at this point in the history
The `sindri lint` file paths logged for Circomspect output were rewriting the
paths relative to the project's root directory on the host system, but the
project is mounted at `/sindri/` inside of the container so this was resulting
in spurious `../../../` prefixes on the file paths. This adjusts how we rewrite
the paths differently based on whether or not the command is being run in
Docker.

Merges #79
  • Loading branch information
sangaline authored Feb 24, 2024
1 parent 3b6dbd9 commit 74b0dfc
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 15 deletions.
11 changes: 7 additions & 4 deletions src/cli/lint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,17 +195,18 @@ export const lintCommand = new Command()
);
const sarifFile = path.join(
"/",
"/tmp/",
"tmp",
"sindri",
`circomspect-${randomUUID()}.sarif`,
);
let sarif: SarifLog | undefined;
let ranInDocker: boolean;
try {
const circuitPath: string =
"circuitPath" in sindriJson && sindriJson.circuitPath
? (sindriJson.circuitPath as string)
: "circuit.circom";
const code = await execCommand(
const { method } = await execCommand(
"circomspect",
["--level", "INFO", "--sarif-file", sarifFile, circuitPath],
{
Expand All @@ -215,7 +216,8 @@ export const lintCommand = new Command()
tty: false,
},
);
if (code !== null) {
ranInDocker = method === "docker";
if (method !== null) {
sindri.logger.debug("Parsing Circomspect SARIF results.");
const sarifContent = readFileSync(sarifFile, {
encoding: "utf-8",
Expand Down Expand Up @@ -297,8 +299,9 @@ export const lintCommand = new Command()
sindri.logger.debug(result, "Missing Circomspect result fields:");
return;
}

const filePath = path.relative(
rootDirectory,
ranInDocker ? "/sindri/" : rootDirectory,
result.locations[0].physicalLocation.artifactLocation.uri.replace(
/^file:\/\//,
"",
Expand Down
30 changes: 19 additions & 11 deletions src/cli/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,9 @@ export async function execCommand(
tag?: string;
tty?: boolean;
},
): Promise<number | null> {
): Promise<
{ code: number; method: "docker" | "local" } | { code: null; method: null }
> {
// Try using a local command first (unless `SINDRI_FORCE_DOCKER` is set).
if (isTruthy(process.env.SINDRI_FORCE_DOCKER ?? "false")) {
logger?.debug(
Expand All @@ -128,7 +130,10 @@ export async function execCommand(
);
} else if (await checkCommandExists(command)) {
logger?.debug(`Executing the "${command}" command locally.`);
return await execLocalCommand(command, args, { cwd, logger, tty });
return {
code: await execLocalCommand(command, args, { cwd, logger, tty }),
method: "local",
};
} else {
logger?.debug(
`The "${command}" command was not found locally, trying Docker instead.`,
Expand All @@ -138,21 +143,24 @@ export async function execCommand(
// Fall back to using Docker if possible.
if (await checkDockerAvailability(logger)) {
logger?.debug(`Executing the "${command}" command in a Docker container.`);
return await execDockerCommand(command, args, {
cwd,
docker,
logger,
rootDirectory,
tag,
tty,
});
return {
code: await execDockerCommand(command, args, {
cwd,
docker,
logger,
rootDirectory,
tag,
tty,
}),
method: "docker",
};
}

// There's no way to run the command.
logger?.debug(
`The "${command}" command is not available locally or in Docker.`,
);
return null;
return { code: null, method: null };
}

/**
Expand Down

0 comments on commit 74b0dfc

Please sign in to comment.