-
Notifications
You must be signed in to change notification settings - Fork 11.9k
feat(@angular/cli): allow using Deno as package manager #30948
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
base: main
Are you sure you want to change the base?
Conversation
|
@csvn, can you please rebase? Thanks. |
This commits configures CI to run Deno E2E tests.
93e7cb3 to
38470cb
Compare
|
Sorry for the delay, rebased now @alan-agius4 🙂 |
|
Looks like Deno has a bug. And the install is failing with |
|
|
||
| if (name === PackageManager.Deno) { | ||
| // Deno CLI outputs "deno 2.4.4" | ||
| return version.replace('deno ', ''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to have changed.
deno --version
deno 2.5.6 (stable, release, x86_64-unknown-linux-gnu)
v8 14.0.365.5-rusty
typescript 5.9.2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... maybe this differs by platform? For me on Windows I get a shorter version output with -v, and a more verbose one with --version. That's why I added the ternary to switch to the -v flag on L224:
| const versionArg = name !== PackageManager.Deno ? '--version' : '-v'; |
> PS C:\Users\csvn\dev\angular-cli> deno --version
deno 2.5.6 (stable, release, x86_64-pc-windows-msvc)
v8 14.0.365.5-rusty
typescript 5.9.2
> PS C:\Users\csvn\dev\angular-cli> deno -v
deno 2.5.6
I used the shorter one (-v) to simplify parsing the version correctly. But I could perhaps change to a regex to extract the version when using --version? Something along the lines of /deno (?<version>\S{5,})/.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't looked into the E2E test errors yet, so I don't know why those fail. But I know it works to create a new Angular project with Deno like this:
> PS C:\Users\ChristianSvensson\Dev\_personal> deno run -Ar npm:@angular/cli new test-project --skip-install --test-runner karma
✔ Which stylesheet system would you like to use? CSS [ https://developer.mozilla.org/docs/Web/CSS]
✔ Do you want to enable Server-Side Rendering (SSR) and Static Site Generation (SSG/Prerendering)? No
✔ Which AI tools do you want to configure with Angular best practices? https://angular.dev/ai/develop-with-ai None
CREATE test-project/angular.json (2166 bytes)
...[snip]...
CREATE test-project/public/favicon.ico (15086 bytes)
Successfully initialized git.
> PS C:\Users\ChristianSvensson\Dev\_personal>
| 'yarn': '1.22.22', | ||
| 'pnpm': '10.17.1', | ||
| 'bun': '1.3.2', | ||
| 'deno': '2.4.4', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you try to update to 2.5.6 to see if the Deno install issue has been addressed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in 4d0947a.
ac20240 to
f26427e
Compare
|
I missed scope in my commits, force pushed to correct commit messages and address failing |
|
Hi, any update? We would like to switch to Deno, due to the current Shai-Hulud attacks. |
But why? |
I think there's two reasons for this.
The two mitigations above helps prevent packages running code on install, and reading from env. I think these were the ways the Shai-Hulud NPM worm used to steal secrets? I tried reading about Possibly the reason anyway I think. 🙂 |
|
Using Deno or other package managers will not inherently prevent the package infected by |
True, it won't stop any environment variables used by an application. But it can prevent prevent reading of a NPM token with publish access when running an application (
No defense is 100%, it's helpful to have multiple layers. Using
I only think this is true when running Angular CLI via I don't mean to go into a debate in this PR. But I typically try to run with permissions I need (and not |
|
You are correct, but to clarify, this pull request specifically focuses on adding support for the Deno package manager, not the Deno runtime. However, the runtime should still function because it is designed to be compatible with the Node.js runtime. I pushed a commit to the PR to fix the issue with the json parsing issues during installation, however now the installation is failing due to missing permissions that are needed to create the |
Well... when running the CLI, it might have to spawn subprocesses though. To run package manager installs, to run build tools like esbuild, etc.. And the second any subprocesses can be spawned, all the protections from deno's permission system are kind of out the window. It's very situational that deno's system helps and this isn't really one of those situations. As @alan-agius4 mentioned, you definitely should consider using a package manager that doesn't run install scripts by default and has a per-package opt-in (e.g. P.S.: Also, more generally, please don't put credentials with write access into your shell. If at all possible, keep those out of your dev machines entirely. If you really need them on your dev machine, only source them where they are needed and consider using something that doesn't store them in plaintext on disk (e.g. the |
I took a look, and it seems there's currently no flag to pass to Created denoland/deno#31498 to see if that's something they want to support. It would be possible to replace |
Yes, only workers preserve the current permissions, child processes are subject to privilege escalation. Though I sometimes run code after adding new dependencies without any permissions just to check what they're trying to access.
You're right, it's not possible to limit permissions to "your own code" or specific packages. Using as few secrets as possible in application code with as few permissions as possible is always a good idea. Though it's rare these days to not require keys for third-party vendors/API's, e.g. Auth (Clerk, Auth0, Okta) when building an app.
Good advice 👍, and yeah we for instance only have
Well, even in containers if a dependency is infected it may secrets/credentials and send them to an outside party from inside the container 😅 As you say with defense in depth, no single strategy covers everything. 🙂 |
|
Pushed a fix for the lint error.
I tried running |
PR Checklist
Please check to confirm your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #30947
What is the new behavior?
It's possible to use
denoto install packages, similar tobun.Does this PR introduce a breaking change?
Other information