Skip to content
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

Allow ng build to put files in the outdir and levels higher than the outdir #8122

Closed
johnpapa opened this issue Oct 19, 2017 · 20 comments · Fixed by #8123
Closed

Allow ng build to put files in the outdir and levels higher than the outdir #8122

johnpapa opened this issue Oct 19, 2017 · 20 comments · Fixed by #8123
Assignees
Labels
P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@johnpapa
Copy link
Contributor

Bug Report or Feature Request (mark with an x)

- [ x ] bug report -> please search issues before submitting
- [ ] feature request

Versions.

@angular/cli: 1.4.4+
node: 6.11.4
os: darwin x64

Repro steps.

See the project here https://github.com/Azure-Samples/angular-cosmosdb/tree/develop in the develop branch.

Notice that I set the outdir and root like this

      "root": "src/client",
      "outDir": "dist/publicweb",

Now notice I have assets that I want to end up in the folder one level higher than the outdir.

"assets": [
        {
          "glob": "**/*.*",
          "input": "../server/",
          "output": "../"
        },
      ],

When i run this I get an error saying I can't do this. This started in 1.4.4. It works in 1.4.3

Desired functionality.

I would like this to work so I can deploy both an express server from src/server/** and the client app , ending up in the outdir, where the node/express server serves everything in a folder below it. See below:

/dist/index.js // my express server
/dist/publicweb // the folder with my angular client code

Mention any other details that might be useful.

I had a chat with @Brocco about this at AngularMix ... I'm hoping we can find a way to do this where it is allowed to go up the outdir as long as it doesn't go past the project root.

@hansl
Copy link
Contributor

hansl commented Oct 20, 2017

This is done so that people don't overwrite their app by mistake (or a malicious app overwrites system files). I'm not sure what the proper way to fix it. Preventing mistakes is relative easy; have a flag that the user needs to specify to true. Preventing malicious apps from outputting your .bashrc while allowing people to put files outside your output dir is more difficult.

I'll think about a proper solution and come to it.

The PR that added this check is #7778

@filipesilva
Copy link
Contributor

@hansl I think a good compromise is a flag on the asset definition:

"assets": [
        {
          "glob": "**/*.*",
          "input": "../server/",
          "output": "../",
          "disablePathCheck": true
        },
      ],

WDYT?

@filipesilva filipesilva self-assigned this Oct 20, 2017
@filipesilva filipesilva added feature: aot P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent labels Oct 20, 2017
@elvisbegovic
Copy link
Contributor

7days ago I asked it here #7773 (comment)

"disablePathCheck": true seems easy but i will never find this option quickly so don't forget improve this error message

"An asset cannot be written to a location outside of the output path. If you know what you're doing add "disablePathCheck": true in assets[] of your config file."

@hansl
Copy link
Contributor

hansl commented Oct 20, 2017

So two points:

  1. I will not disable all checks. It would be disastrous to disable writing outside your project.
  2. I agree that whatever we choose we need to make the error message easy for the user to follow.

@johnpapa
Copy link
Contributor Author

I agree on #1. There should be a check.

Perhaps there is a way to allow going up the chain by writing logic that checks that the output does not have more dots up the chain than the the outDir has slashes.

"assets": [
      "outDir": "dist/publicweb",
        {
          "glob": "**/*.*",
          "input": "../server/",
          "output": "../",
        },
      ],

@clydin
Copy link
Member

clydin commented Oct 20, 2017

Something to consider as well is that the use cases shown so far (universal, multi-apps, and translations) are all really dealing with deployment staging rather than app asset copying. The CLI doesn’t currently have a method to handle the post-build staging step so the asset configuration is being novelly used/abused to provide the functionality.
Deployment staging can be achieved currently with shell/npm scripts. However, as universal and multi-language apps become more common due to the prerequisite tools becoming more stable, a more integrated staging feature set could be every useful moving forward.

@hansl
Copy link
Contributor

hansl commented Oct 22, 2017

Deployment is an entirely separate story though. I agree we need a proper solution for deployment, but it requires design and planning. This is good enough for most people in the meantime.

As far as option goes, I'll try to keep a positive option instead of double negative (allow or enable instead of disable).

@hansl
Copy link
Contributor

hansl commented Oct 22, 2017

What do you think of allowOutsideOutDir?

@johnpapa
Copy link
Contributor Author

that works.

I agree deployment is different than a build. But a build helps put things in the right place, so we can run them locally how we wish. I do think the CLI is a valid place to do simple copy logic like this ... once it goes beyond copying, it should be a separate thing IMO.

I'm feeling this out ... trying not to introduce yet another thing into the process. I can show you sometime if that helps.

hansl added a commit to hansl/angular-cli that referenced this issue Oct 22, 2017
@johnpapa
Copy link
Contributor Author

@hansl thanks for the fix. When do you expect this to be merged?

@elvisbegovic
Copy link
Contributor

dammage wasn't in 1.5.0

@dabears318
Copy link

also experiencing this issue as of 1.4.4

@johnpapa thank you for all your work/tutorials with angular cli, node & azure builds.

any idea on a timeline for merge? @hansl

@MaximeAnsquer
Copy link

Hello, could you help me use this feature please? I don't know where I'm supposed to put this " allowOutsideOutDir ". Thank you very much!

@krajcovic46
Copy link

@MaximeAnsquer Hello, you can find your answer here.

@johnpapa
Copy link
Contributor Author

johnpapa commented Dec 3, 2017

Thank you @hansl !

@Hypenate
Copy link

Hypenate commented Mar 8, 2018

I was wondering if you could do the same for a select number of files.

This takes the whole folder with all files
{ "glob": "**/*", "input": "../node_modules/some-package/images", "output": "./some-place/" }

Could you limit it with something like:
Ends with foo
{ "glob": "**/*", "input": "../node_modules/some-package/images/*foo", "output": "./some-place/" }
Begins with bar
{ "glob": "**/*", "input": "../node_modules/some-package/images/bar*", "output": "./some-place/" }

@Kunepro
Copy link

Kunepro commented Aug 14, 2019

If I read this tread there seems to be a solution, if I check this link instead everything mentioned was discarded.

Currently I have a build problem with this and CloudFoundry.
For the required configuration to deploy with staticfile buildpack I need something resembling this structure:

  • /dist/folder-with-actual-angular-build-files/...the-build-files
  • /dist/manifest.yml,
  • /dist/nginx/..../conf.file
  • /dist/Staticfile

The angular build must be inside a folder that Staticfile can specify, here "folder-with-actual-angular-build-files" and not in the same folder as the configuration files.
That's why, like OP, I would like to move some of the assets in a folder outside, parent of, the actual Angular build.
I understand that going outside the outputPath is not allowed for not mentioned security reason.

But what if...

We leave the output path as it is, so in this case "outputPath": "dist/".
Then, we add an optional parameter to specify where the angular compiled build should be, relative to the outputPath. So as per my example: "projectBuild": "folder-with-actual-angular-build-files/". If unspecified the dist folder will be used.
At this point I can just specify to add all my meta files in the root of outputPat and I get the folder structure I want, without going outside of the outputPath/dist.
For example for nginx would be:
{ "glob": "**/*", "input": "apps/my-sweet-app/src/deployment/nginx", "output": "/nginx/" }
I just add a setting to actually nest the angular build in a folder in the outputPath to leave space for meta-files for deployment. Would this be a satisfying solution for the security concerns?

@radvansky-tomas
Copy link

Any update on this ?

@Kunepro
Copy link

Kunepro commented Aug 19, 2019

Any update on this ?

Not yet and even then don't expect it soon.
There's a workaround anyway.
When you try to add some files in the parent folder you get the security error message to prevent you from doing so, however this is a very simple check against "../" at the beginning of the string, the solution is to replace the path to: "./../etcetera", this way you can still move the configuration files in the parent folder and ignore the security check.

I have to say that it wasn't nice to remove the topic's feature without providing an alternative solution.
Please do not fix this until you can actually provide one.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.