Hi Konrad, > On 22 Sep 2021, at 09:04, Konrad Windszus <[email protected]> wrote: > > I have some remarks though: > > 1. We should disable external entity processing in VaultContentXmlReader > (https://sonarcloud.io/project/issues?id=apache_sling-scriptingbundle-maven-plugin&open=AXwCdaTmnSy41Wx6i_Vn&resolved=false&types=VULNERABILITY > > <https://sonarcloud.io/project/issues?id=apache_sling-scriptingbundle-maven-plugin&open=AXwCdaTmnSy41Wx6i_Vn&resolved=false&types=VULNERABILITY>)
I think that this is a false positive, unless I miss something really obvious. External entities shouldn’t be able to load anything [0]. If I’m correct, then we should mark the issue as such in SonarCloud. > 2. I read the documentation at > https://github.com/apache/sling-scriptingbundle-maven-plugin/blob/master/src/site/markdown/bnd.md#working-with-filevault-content-package-projects > > <https://github.com/apache/sling-scriptingbundle-maven-plugin/blob/master/src/site/markdown/bnd.md#working-with-filevault-content-package-projects> > and tried the example at > https://github.com/apache/sling-org-apache-sling-scripting-bundle-tracker-it > <https://github.com/apache/sling-org-apache-sling-scripting-bundle-tracker-it>, > IIUC this excludes the scripts from the content package by just not listing > the root node path in the filter.xml. On the other hand, everything below > target/classes ends up in the bundle jar. In reality this is too simplified > as often the resource type node folders contain additional information which > would be lost that way like additional properties (not reflected in the > bundle jar metadata) or configuration structures. Also profiles used for > deployment need to be adjusted. I would instead recommend to keep the package > as is (i.e. make it still contain the resource type nodes even if redundant) > and rely on the service ranking to make the bundled (precompiled) scripts > take precedence? Do you see a drawback with that approach? You are right that the example is simplistic. I wanted to emphasise that the /apps scripts are not needed any more when working with precompiled bundled scripts. We can indeed allow the content package to contain all nodes (including the ones covered by [1]) and explain that the bundle providing the precompiled bundled scripts has priority over scripts from the resource tree for the same resource types/selectors. Thanks, Radu [0] - https://github.com/apache/sling-scriptingbundle-maven-plugin/blob/scriptingbundle-maven-plugin-0.5.0/src/main/java/org/apache/sling/scriptingbundle/plugin/processor/filevault/VaultContentXmlReader.java#L57-L58 <https://github.com/apache/sling-scriptingbundle-maven-plugin/blob/scriptingbundle-maven-plugin-0.5.0/src/main/java/org/apache/sling/scriptingbundle/plugin/processor/filevault/VaultContentXmlReader.java#L57-L58> [1] - https://github.com/apache/sling-org-apache-sling-scripting-bundle-tracker-it/blob/master/examples/org-apache-sling-scripting-content-package-with-bundle-attached/src/main/content/META-INF/vault/filter.xml#L23 <https://github.com/apache/sling-org-apache-sling-scripting-bundle-tracker-it/blob/master/examples/org-apache-sling-scripting-content-package-with-bundle-attached/src/main/content/META-INF/vault/filter.xml#L23>
