`moduleName` absolute path leakage regression

[chancancode]

Previously:

• https://github.com/embroider-build/embroider/issues/141
• https://github.com/embroider-build/embroider/pull/243

With the recent change to put the rewritten app/packages into node_modules/.embroider/*, it seems like the previous fix was accidentally defeated. With addon provided templates at least, I am seeing moduleName (in production build) regressed to something like /Users/godfrey/code/discourse/app/assets/javascripts/discourse/node_modules/.embroider/rewritten-packages/.../...hbs, which is 1) very long and 2) potentially sensitive and 3) causes otherwise unnecessary cache bust.

I am aware that there are some ongoing discussions around the meaning/utility of moduleName in the gjs world, and potential plan to deprecate it, but imo this issue is unrelated. I think because the fix in #243 was quite targeted at the implementation detail at the time and lacks end-to-end integration test, with the recent refactor, things have escaped the spirit of the original fix.

[chancancode]

@ef4 is this just an oversight or is this absolute path actually used by something? (in that case, that would be related to #1647)

[ef4]

The absolute path is not used by anything.

A newer attempt at fixing this was https://github.com/embroider-build/embroider/pull/1517, and there's discussion there about what would be required to actually fix it.

If your concern is only build stability and data-hiding, I'm also fine with adding a flag to entirely remove moduleName. It would be good to double-check exactly what changes in the dev experience without it. Only ember-inspector? Or are there other debug messages that rely on it?

[chancancode]

Yeah, I actually contemplated adding a post process step to get ride of it, so if you are happy to have it live here I figured it doesn’t hurt anything

[chancancode]

As for the middle ground of keeping it around, if it’s not used by anything we just have to adjust it so a human can look at it and plausibly make sense of the string right

[ef4]

As for the middle ground of keeping it around, if it’s not used by anything we just have to adjust it so a human can look at it and plausibly make sense of the string right

Yes, except it can also get seen by people's custom AST transforms. But right now those are already presumably seeing the absolute path and so don't have perfect backward compatibility anyway, so that would be no worse than now.

We do actually have test coverage that an AST transform sees the backward-compatible name here:

https://github.com/embroider-build/embroider/blob/cd8d1be2f1a8071d5d6b27d3ce63f052758d4c46/tests/scenarios/compat-stage2-test.ts#L675-L681

And that is passing. I think perhaps the difference is app vs addon code.

[amk221]

I just spotted this in our production builds, isn't it a bit of a security concern?

[NullVoxPopuli]

maybe a privacy one if folks deploy from their own machines

[kiwi-josh]

I have also just spotted this in our production builds - worried me seeing our build-boxes name + path pop up in our sentry logs