Skip to content

[vectors/color] Content Security Policy use of eval / Function #497

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

Open
dennemark opened this issue Apr 2, 2025 · 12 comments
Open

[vectors/color] Content Security Policy use of eval / Function #497

dennemark opened this issue Apr 2, 2025 · 12 comments

Comments

@dennemark
Copy link

Hi @postspectacular ,
not sure how you think about this issue, since it goes bit deeper into your vector and color functions:
I am using @thi.ng/color in frontend and it imports clamp and later on a compileHOF function that calls new Function. Now if I want to set Content Security Policy on my backend, my browser complains that I am using 'eval/Function'. To avoid this I have to state on 'script-src': 'unsafe-eval'. While this is ok for thi.ng library, it however opens up possibilities for other attack vectors.
So I was wondering, if it would be possible to actually compile these functions for the shipped code, since then the compileHOF would not be necessary anymore.
I think it is also a problem of thi.ng/vectors, since compileHOF is within this package.

Let me know what you think.

@postspectacular
Copy link
Member

Hi @dennemark — wow, I wasn't aware that this has such an impact, and it seems switching to actual source code generation seems to be the only safe solution to this problem, But:

  1. It's only possible in some cases (e.g. the vectors package), but will also lead to an explosion in package size (I'm guessing at least a magnitude larger). It's really annoying that this CSP setting cannot be defined more fine grained.
  2. There're a few other packages where there's no easy way round dynamic code generation (e.g. custom color types in thi.ng/color and the various pixel formats in thi.ng/pixel) and I'm not quite sure how to solve these...

Will have to research this more, please bear with me... 🤔 🙏

@dennemark
Copy link
Author

dennemark commented Apr 3, 2025

I see.. that can be pretty annoying. CSP was a rabbit hole for me too the last few days. Changing our backend and the prior backend just did not use it. I am not even sure, if this will be the only situation where i will face unsafe-eval, since we also have some wasm modules.
I guess compileHOF could not be replaced to more explicit function, since there are too many cases where compileHOF is used, right?
So far I also did not find a safer way to use it. would have been nice to have kind of a nonce value for eval, even though it would be challenging to channel it through to the function.

Concerning that wasm also is unsafe, I feel I mainly have to make sure that we use trustworthy third-party modules. Other than that inline scripts are only allowed via nonces, so that gives safety, I assume.

postspectacular added a commit that referenced this issue Apr 3, 2025
- this new code generator consumes existing vector fns and re-emits them as source files
- initial proof-of-concept only, WIP
@postspectacular
Copy link
Member

postspectacular commented Apr 3, 2025

@dennemark I just imported a new proof-of-concept code generator which consumes existing vector fns and re-emits them as source files into a new directory. I think this will work, in principle, for the vectors package, but as mentioned earlier, the resulting package size will be MUCH bigger, but there're also some positive aspects, e.g:

  • We can now properly add doc strings for all vector ops
  • The new file structure should help with dead code elimination when using a bundler
  • None of the unused versions need to be code generated at runtime, so also improving startup time

Here's some example output for just the add() function (resulting in 5 source files now):

import type { MultiVecOpVV } from "./api.js";
import { vop } from "./vop.js";
import { add2 } from "./add2.js";
import { add3 } from "./add3.js";
import { add4 } from "./add4.js";

/**
 * Multi-method. Componentwise nD vector addition. Auto-delegates to optimized
 * versions where possible.
 *
 * @param o - output vector
 * @param a - input vector
 * @param b - input vector
 */
export const add: MultiVecOpVV = vop(1);
add.default((o,a,b) => {
!o && (o=a);for(let i=a.length;--i>=0;) {o[i]=a[i]+b[i];}return o;
});
add.add(2, add2);
add.add(3, add3);
add.add(4, add4);
import type { VecOpVV } from "./api.js";

/**
 * Componentwise 2D vector addition.
 *
 * @param o - output vector
 * @param a - input vector
 * @param b - input vector
 */
export const add2: VecOpVV = (o,a,b) => {
!o && (o=a);o[0]=a[0]+b[0];o[1]=a[1]+b[1];return o;
};
import type { VecOpVV } from "./api.js";

/**
 * Componentwise 3D vector addition.
 *
 * @param o - output vector
 * @param a - input vector
 * @param b - input vector
 */
export const add3: VecOpVV = (o,a,b) => {
!o && (o=a);o[0]=a[0]+b[0];o[1]=a[1]+b[1];o[2]=a[2]+b[2];return o;
};
import type { VecOpVV } from "./api.js";

/**
 * Componentwise 4D vector addition.
 *
 * @param o - output vector
 * @param a - input vector
 * @param b - input vector
 */
export const add4: VecOpVV = (o,a,b) => {
!o && (o=a);o[0]=a[0]+b[0];o[1]=a[1]+b[1];o[2]=a[2]+b[2];o[3]=a[3]+b[3];return o;
};

I will continue with this over the coming weeks, but it's a major undertaking and I still haven't looked deeper into the impact of the color & pixel packages...

@dennemark
Copy link
Author

dennemark commented Apr 3, 2025

Super nice!
Also this is great!

We can now properly add doc strings for all vector ops

Wondering how the package size impact will be. I guess having two different imports is not a good solution right? So one could choose i.e. @thi.ng/vectors/opt as import and would get the better bundle size.
Your call, how you want to continue!

@postspectacular
Copy link
Member

@dennemark made a lot of progress on the codegen/converter, now already more than 1/3 updated. I think the larger bundle size only has an impact when using full pkg imports without a bundler involved, but otherwise I'm actually getting smaller file sizes for some of the examples I've already tested...

Btw. I don't think having two versions of the vectors pkg is feasible, neither from a usage nor maintenance point of view... The pkg is used as dependency for many others, and these packages will need to make a decision — too much complexity, easier to just have one...

@dennemark
Copy link
Author

Nice sounds good! Does it make a huge difference for importing vector or colors pkg? Since it is quite convenient to import them via import * as vec .... But I could also change those. I guess it is also possible to do import { vec2 } as vec, not sure though right now.

But sounds like a good approach then!

@postspectacular
Copy link
Member

postspectacular commented Apr 7, 2025

@dennemark If you're using a bundler, then package-level imports like import * as vec ... would be absolutely fine (better than current) since they will undergo tree shaking. Such imports are only a massive potential waste of bandwidth for non-bundling workflows, e.g. by doing something like this from a CDN:

import * as vec from "https://esm.run/@thi.ng/vectors";

...but for this kind of usage, such package-level imports are a bad form for any larger package, and the generally best practice there is to always handpick the imports you actually care about, e.g.

import { distSq2 } from "https://esm.run/@thi.ng/vectors/distsq";

One thing I'll look into (at a later stage) is to maybe split up the vectors package into 2D, 3D, 4D and nD versions, but for that we will first need this codegen business fully completed...

postspectacular added a commit that referenced this issue Apr 8, 2025
- update FnSpec
- add boolean & strided vector ops
@dennemark
Copy link
Author

Thanks for the clarification!

Concerning the vectors packages split up... will it have some advantages to split it up? Since it currently also works well no?

postspectacular added a commit that referenced this issue Apr 9, 2025
- add various random ops
- add most strided ops
- update doc string handling/interpolation
- add support for dimensional variations in doc links
- collect total file sizes
@postspectacular
Copy link
Member

@dennemark I'm only looking into splitting up as an option due to the (quite!) larger package size (currently, 280KB of source code — also caused by a lot more and largely duplicated doc strings present). I really want to keep the package usable for non-bundling workflows (btw. just out of interest: are you using any bundlers yourself?).

There're quite a few possibilities for splitting, but my current thinking is:

  • thi.ng/vec-api: common/shared vector types & interfaces (e.g. mostly what is in api.ts)
  • thi.ng/vec2: only 2D vector ops, constants, the Vec2 class wrapper and the 2D VecAPI impl
  • thi.ng/vec3: same for 3D vector ops, etc.
  • thi.ng/vec4: same 4D vector ops...
  • thi.ng/vec-nd: generic n-dimensional vector ops & gvec proxy wrapper
  • thi.ng/vectors: polymorphic/extensible facades/wrappers for all

I could also imagine both the thi.ng/vec-nd and thi.ng/vectors packages to be kept as one, but having the plain, non-polymorphic nD functions separate would be beneficial...

The vec2/3/4 & vec-nd packages would have 99% the same structure, but only include the size-optimized functions and only differ in operations which are only available for some dimensions (e.g. perpendicularCW() is only for 2D, orthoNormal3() only available for 3D etc.)

The main benefit of this structure would be smaller packages (with less dependencies) when only using the 2D/3D/4D/nD features individually. E.g. thi.ng/geom would only require the vec2/3 packages. thi.ng/color would just need the vec4 package etc.

Again, this is all needing some experimentation & testing, but I'm now mostly done with the codegen and will concentrate on this next...

@dennemark
Copy link
Author

Ah wow, i am so used to bundlers now, that I did not expect you paying so much attention on non-bundling workflow. I use vite and just strip away all comments or docs. I remember in 2017 or so I did not program for the web for years and started importing via script tags and was happy that it worked.
Though for some other library of my own I used to publish a non-minified version, so it is easy to debug.

Our main app where we use thi.ng is a SPA with a login, so bundle size does not matter too much. Even though it could improve loading times here and there..

I like the approach of combinig the sub vec packages in thi.ng/vectors so it would not be really a breaking change. But also it could clarify usage a bit more clearly. So when is vec2 or vec3 used. Good idea.

Close this issue when you feel comfortable about it!
Thanks a lot!

postspectacular added a commit that referenced this issue Apr 14, 2025
BREAKING CHANGE: Due to major refactoring & restructuring related to #497, some direct imports & function names have changed

- replace former codegen approach with higher-order functions
- split up various operations into more granular source files
- add doc strings for almost all functions (🎉 — lack of was a former major criticism of this pkg)
- update/remove deps
postspectacular added a commit that referenced this issue Apr 14, 2025
BREAKING CHANGE: Refactoring & restructuring related to #497

- replace former codegen approach with higher-order functions
- add new`defMath()` &  `defMathN()` impls
postspectacular added a commit that referenced this issue Apr 14, 2025
* feature/vec-splitup: (24 commits)
  test(color): update `random` test consts
  fix(matrices): minor update `identity()`
  refactor(shader-ast-js): internal updates (imports & renames)
  refactor(imgui): minor internal updates (imports)
  refactor(color): minor internal updates (imports)
  refactor(tsne): minor internal updates
  refactor(matrices): update to remove dynamic codegen (#497)
  feat(vectors): replace 99% of vector ops with new  versions
  build: update yarn lock
  feat(vectors2): copy tests from old pkg
  feat(vectors2): add random ops for strided vectors
  feat(vectors2): add randNormS()
  fix(vectors2): fix normalizeS2/3/4()
  fix(vectors2): fix fit3/4
  feat(vectors2): add/update various ops
  chore: remove obsolete/experimental vector pkgs
  feat(vectors2): import as new pkg (temporary only, will replace vectors pkg)
  feat(vec3): add/update/replace/rename various ops
  refactor(vec2): add/update/merge various ops/files
  feat(vec2): add/update ops
  ...
@postspectacular
Copy link
Member

Hi @dennemark — just a quick update to say that I've already replaced the vectors package on the develop branch and after lots of deep experimentation have decided NOT to split up the package. There will a few (minor) breaking changes, mainly because of enforcing more consistent naming and more granularity in some source files (therefore possibly changed imports, though only if you use direct ones for individual fns...). All in all, I've managed to keep the impact on users to a bare minimum (likely unnoticeable for most), even though it's pretty much a complete rewrite of the entire package...

In terms of file size impact: The FULL minified pkg bundle is now 56.4KB vs previously 48.5KB, however the code density has improved and the brotli-compressed pkg size is only 1KB larger, which I found absolutely incredible! 🎉

Also — more importantly — many of the examples in the repo are showing between 5-25% smaller result bundle sizes, some also have become slightly larger, but so far I found the most by only ~2%...

Related to this change: I've also updated the color & matrices packages to be free from dynamic code generation now! The only packages still using new Function(...) are the following, but for those they're unavoidable and dynamic code generation is a core feature:

  • thi.ng/pixel (custom pixel format definition/compilation)
  • thi.ng/pixel-convolve (custom image convolution kernel compilation)
  • thi.ng/shader-ast-js (Shader AST to JavaScript compilation)

I will do more testing over the coming days, then release new version(s) asap...

@dennemark
Copy link
Author

Wow that is amazing! Nice bundle size still is small or even smaller than before!
Thanks a lot for your huge efforts! Will be trying it soon! From my side we can close this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants