sentry-cli icon indicating copy to clipboard operation
sentry-cli copied to clipboard

Protect against spawn of CLI for wrong architecture

Open danielkhan opened this issue 3 years ago • 21 comments
trafficstars

Problem

The CLI is rarely used standalone. In most cases, it is executed via intermediates like the Sentry Webpack plugin.

When the CLI is installed via npm, it will run an install script that downloads a binary for the current architecture.

There are scenarios, when the architecture where the CLI eventually runs differs from the one npm install was executed from.

E.g.

  • A Dockerfile that will simply copy the local node_modules into the container
  • Any other deployment methods that will simply ship the bundle to the destination system including node_modules

This can lead to a rather obscure error when the binary is executed/spawned: #0 26.19 Sentry CLI Plugin: spawn Unknown system error -8. I could reproduce this problem by running npm install @sentry/nextjs on an M1 Mac before running the application in Docker (Alpine).

The problem was reported by other users as well and we didn't come up with a solution proposal yet:

  • https://github.com/getsentry/sentry-cli/issues/508

Quick fix

The problem can be solved by running

  • npm uninstall <sentry_module_that_installs_the_CLI_as_dep>
  • followed by npm install <sentry_module_that_installs_the_CLI_as_dep> as part of the deployment on the target system.

For a next.js application running in Docker, this would mean adding the following to the Dockerfile:

RUN npm uninstall -S @sentry/nextjs
RUN npm install -S @sentry/nextjs

# Further commands below, like:
# RUN npm run build

This ensures that the right binary is downloaded for the target system.

Product improvement

Sentry should detect that the binary doesn't match the current architecture and provide an actionable error message to the user. E.g.

Wrong architecture detected while trying to execute the Sentry CLI. Please make sure to do a fresh npm uninstall/install step for @sentry modules on the target system.

danielkhan avatar Aug 31 '22 10:08 danielkhan

We only need to do this for UNIX systems, as for windows .exe should be always compiled for windows. In this case, we can most likely use file sentry-cli --brief or file sentry-cli --mime-type and match on the output.

kamilogorek avatar Aug 31 '22 14:08 kamilogorek

Hi, is there any progress on this issue?

I am still getting the error: Sentry CLI Plugin: spawn Unknown system error -8 when building the nextjs app in the docker node:18-alpine image.

I tried to uninstall the @sentry/nextjs package and install back as you suggested but it did not help.

samueldusek avatar Sep 15 '22 11:09 samueldusek

No, not really. As for your issue, can you run file sentry-cli and/or file node_modules/@sentry/cli/sentry-cli inside your docker image and provide logs?

kamilogorek avatar Sep 15 '22 11:09 kamilogorek

@samueldusek In your Dockerfile, try adding

RUN npm uninstall --save @sentry/nextjs
RUN npm install --save @sentry/nextjs

This works for me as it will make sure that you get the right CLI for the platform your container runs on.

danielkhan avatar Sep 15 '22 11:09 danielkhan

In your Dockerfile, try adding

RUN npm unininstall --save @sentry/nextjs
RUN npm install --save @sentry/nextjs

This works for me as it will make sure that you get the right CLI for the platform your container runs on.

@danielkhan As I mentioned in my first comment. I did try it and it did not help.

samueldusek avatar Sep 15 '22 11:09 samueldusek

@samueldusek what happens if you remove node_modules altogether before spinning up the container?

danielkhan avatar Sep 15 '22 11:09 danielkhan

@kamilogorek This is the output from the file command you suggested.

#22 [deps 15/15] RUN file node_modules/@sentry/cli/sentry-cli
#22 sha256:4cf4b1d85d4779ee88fec1a6be0b8f9a5cb593fd6ec871aef5664aa7e3a5d389
#22 1.481 node_modules/@sentry/cli/sentry-cli: ELF 64-bit LSB executable, ARM aarch64, version 1 (SYSV), statically linked, with debug_info, not stripped
#22 DONE 1.6s

This is how I am trying to install dependencies for my app in the docker file.

COPY package.json yarn.lock ./
RUN yarn install
RUN yarn remove @sentry/nextjs
RUN yarn add @sentry/nextjs

RUN file node_modules/@sentry/cli/sentry-cli

samueldusek avatar Sep 15 '22 12:09 samueldusek

what happens if you remove node_modules altogether before spinning up the container?

@danielkhan The image building process fails do to the error mentioned above when running

RUN yarn next build

So I am not even able to build my app due to the sentry-cli error. 😞

Just to mention.. When I am building the app on my local machine I have no issue.

samueldusek avatar Sep 15 '22 12:09 samueldusek

What happens if you remove Sentry from your local install and then docker exec into the image and run the yarn install manually, @samueldusek?

danielkhan avatar Sep 15 '22 13:09 danielkhan

What happens if you remove Sentry from your local install and then docker exec into the image and run the yarn install manually, @samueldusek?

@danielkhan Well, I tried to remove Sentry from my local install as you suggested and it started to work. But I do not think that it is a solution as I cannot build the app on my local machine anymore.

There should be a solution I can build the app both, localy and in docker as well without any issues. 😞

samueldusek avatar Sep 15 '22 18:09 samueldusek

@samueldusek this is more for the purpose of triaging the problem. If it works in Docker then, this means that the right CLI can be installed and something just prevents this from happening. Would you mind posting the respective Dockerfile?

danielkhan avatar Sep 15 '22 18:09 danielkhan

@samueldusek If you use the similar to the one of the nextjs-example (https://github.com/vercel/next.js/blob/canary/examples/with-docker/Dockerfile), you have to execute

RUN yarn remove @sentry/nextjs
RUN yarn add @sentry/nextjs

after

COPY --from=deps /app/node_modules ./node_modules
COPY . .

If you do it after the RUN yarn install --frozen-lockfile but before the COPY . ., the COPY . . will actually copy your local node_modules and overwrite the one in the container. That could be the reason why you still have that error even after using the remove/add method.

ekrokowski avatar Sep 27 '22 17:09 ekrokowski

@samueldusek If you use the similar to the one of the nextjs-example (https://github.com/vercel/next.js/blob/canary/examples/with-docker/Dockerfile), you have to execute

RUN yarn remove @sentry/nextjs
RUN yarn add @sentry/nextjs

after

COPY --from=deps /app/node_modules ./node_modules
COPY . .

If you do it after the RUN yarn install --frozen-lockfile but before the COPY . ., the COPY . . will actually copy your local node_modules and overwrite the one in the container. That could be the reason why you still have that error even after using the remove/add method.

That fixed my problem while running docker-compose build . The build process was exiting with an error Sentry CLI Plugin: spawn Unknown system error -8

umuralpay avatar Nov 07 '22 12:11 umuralpay

@vladanpaunovic and @kamilogorek is this issue something we can pull back up and discuss if there is a way forward?

smeubank avatar Nov 09 '22 10:11 smeubank

I don't believe there's an easy way out of this, as this is docker build misconfiguration, and we cannot control the user-land. I'm more than happy to hear some suggestions though.

kamilogorek avatar Nov 09 '22 14:11 kamilogorek

Goals:

  1. Don't error out like that by accessing a file that is not for the given arch - at least give the user a proper error message.
  2. Automatically ensure that the right file is being downloaded if it isn't present.

Would it be possible to download and store the respective binaries in a directory structure that contains the arch /cli/<arch>/foo.xyz? With that, checking for the right binary and maybe even downloading the right one just in time should be fairly trivial.

danielkhan avatar Nov 09 '22 14:11 danielkhan

Changing

COPY --from=deps /app/node_modules ./node_modules
COPY . .

to

COPY . .
COPY --from=deps /app/node_modules ./node_modules

fixes the issue and you don't need to remove and install sentry again

umuralpay avatar Nov 09 '22 14:11 umuralpay

Would it be possible to download and store the respective binaries in a directory structure that contains the arch /cli//foo.xyz? With that, checking for the right binary and maybe even downloading the right one just in time should be fairly trivial.

That would definitely be possible, but we have lots of other tools that rely on the fact that sentry-cli{.exe} binary lives in the root of the installed package. And changing it now would definitely be not trivial. We'd have to play around with symlinking correct file, that in itself can be sometimes problematic. It is one of possible solutions though.

kamilogorek avatar Nov 09 '22 15:11 kamilogorek

RUN npm unininstall --save @sentry/nextjs RUN npm install --save @sentry/nextjs

We aware that there's a typo in the first line. It should be

RUN npm uninstall --save @sentry/nextjs RUN npm install --save @sentry/nextjs

LegoKristoffer avatar Nov 17 '22 12:11 LegoKristoffer

Oh, nice catch! Update the comment, thanks

kamilogorek avatar Nov 17 '22 12:11 kamilogorek

Another option, without swapping copy/install is to add .dockerignore file and ignore node_modules there. This way docker won't copy that dir.

#.dockerignore
node_modules

dmitrika avatar Apr 19 '23 13:04 dmitrika

We believe #1836 should likely also resolve this issue. We are closing this issue for now; if anyone continues experiencing this problem, please reopen!

szokeasaurusrex avatar Mar 19 '24 12:03 szokeasaurusrex