Make web push & imgproxy setup optional for local dev

[ekzyis]

Close #357

[wbobeirne]

I think your PR is messed up, it's got 16k lines changed!

[ekzyis]

think your PR is messed up, it's got 16k lines changed!

It was based on a branch for upgrades which was merged into master since. We wanted to avoid merge conflicts since there were a lot of changes in the upgrades branch (as you can see).

But seems like the commit hashes changed since I assumed the changes would just disappear here after the merge.
So I need to re-rebase that branch on master, then all these changes should disappear. Thanks for letting me know!

[brihuang99]

Apologies if this isn't the right place for this issue, but I'm running into some errors after running docker-compose up --build after this pull request had been merged:

app       | > stackernews@0.1.0 dev
app       | > NODE_OPTIONS='--trace-warnings' next dev
app       | 
app       | 
imgproxy  | INFO    [2023-08-08T17:36:42Z] Started /health  request_id=HONuTVcdN_UBswRmVI3sD method=GET client_ip=127.0.0.1
imgproxy  | INFO    [2023-08-08T17:36:42Z] Completed in 54.265µs /health  request_id=HONuTVcdN_UBswRmVI3sD method=GET status=200 client_ip=127.0.0.1
app       | fatal: detected dubious ownership in repository at '/app'
app       | To add an exception for this directory, call:
app       | 
app       |     git config --global --add safe.directory /app
app       | 
app       | - error Failed to load next.config.js, see more info here https://nextjs.org/docs/messages/next-config-error
app       | node:internal/errors:865
app       |   const err = new Error(message);
app       |               ^
app       | 
app       | Error: Command failed: git rev-parse HEAD
app       | fatal: detected dubious ownership in repository at '/app'
app       | To add an exception for this directory, call:
app       | 
app       |     git config --global --add safe.directory /app
app       | 
app       |     at checkExecSyncError (node:child_process:885:11)
app       |     at Object.execSync (node:child_process:957:15)
app       |     at Object.<anonymous> (/app/next.config.js:20:30)
app       |     at Module._compile (node:internal/modules/cjs/loader:1256:14)
app       |     at Module._extensions..js (node:internal/modules/cjs/loader:1310:10)
app       |     at Module.load (node:internal/modules/cjs/loader:1119:32)
app       |     at Module._load (node:internal/modules/cjs/loader:960:12)
app       |     at ModuleWrap.<anonymous> (node:internal/modules/esm/translators:169:29)
app       |     at ModuleJob.run (node:internal/modules/esm/module_job:194:25) {
app       |   status: 128,
app       |   signal: null,
app       |   output: [
app       |     null,
app       |     Buffer(0) [Uint8Array] [],
app       |     Buffer(152) [Uint8Array] [
app       |       102,  97, 116,  97, 108,  58,  32, 100, 101, 116, 101,  99,
app       |       116, 101, 100,  32, 100, 117,  98, 105, 111, 117, 115,  32,
app       |       111, 119, 110, 101, 114, 115, 104, 105, 112,  32, 105, 110,
app       |        32, 114, 101, 112, 111, 115, 105, 116, 111, 114, 121,  32,
app       |        97, 116,  32,  39,  47,  97, 112, 112,  39,  10,  84, 111,
app       |        32,  97, 100, 100,  32,  97, 110,  32, 101, 120,  99, 101,
app       |       112, 116, 105, 111, 110,  32, 102, 111, 114,  32, 116, 104,
app       |       105, 115,  32, 100, 105, 114, 101,  99, 116, 111, 114, 121,
app       |        44,  32,  99,  97,
app       |       ... 52 more items
app       |     ]
app       |   ],
app       |   pid: 441,
app       |   stdout: Buffer(0) [Uint8Array] [],
app       |   stderr: Buffer(152) [Uint8Array] [
app       |     102,  97, 116,  97, 108,  58,  32, 100, 101, 116, 101,  99,
app       |     116, 101, 100,  32, 100, 117,  98, 105, 111, 117, 115,  32,
app       |     111, 119, 110, 101, 114, 115, 104, 105, 112,  32, 105, 110,
app       |      32, 114, 101, 112, 111, 115, 105, 116, 111, 114, 121,  32,
app       |      97, 116,  32,  39,  47,  97, 112, 112,  39,  10,  84, 111,
app       |      32,  97, 100, 100,  32,  97, 110,  32, 101, 120,  99, 101,
app       |     112, 116, 105, 111, 110,  32, 102, 111, 114,  32, 116, 104,
app       |     105, 115,  32, 100, 105, 114, 101,  99, 116, 111, 114, 121,
app       |      44,  32,  99,  97,
app       |     ... 52 more items
app       |   ]
app       | }
app       | 
app       | Node.js v18.17.0
app       | 
dependency failed to start: container app exited (1)

Any idea what's going on here? Thanks.

[huumn]

This error means the user running the next dev command does not own the .git folder afaik.

It's strange that this happened after that commit because it shouldn't have an affect on something like this. I'll try to reproduce locally.

[huumn]

So this started up without problem for me. I'd guess there's something weird going on with the permissions of your working directory (the one you cloned into, /app in the docker container) or the /app/.git directory within it and the user in docker. That's basically what this means: fatal: detected dubious ownership in repository at '/app'

The permissions of my folders are: drwxr-xr-x keyan staff if that helps at all.

If you just want to cure the symptom, you can set const commitHash = undefined in next.config.js and you should be good to go.

[ekzyis]

I get the same error since SN includes the version in the footer. So this shouldn't be new afaict.

I worked around this a while ago with this is patch:

diff --git a/next.config.js b/next.config.js
index 35c80cf..e4c28d6 100644
--- a/next.config.js
+++ b/next.config.js
@@ -15,13 +15,13 @@ const corsHeaders = [
 ]

 // XXX this fragile ... eb could change the version label ... but it works for now
-const commitHash = isProd
-  ? Object.keys(require('/opt/elasticbeanstalk/deployment/app_version_manifest.json').RuntimeSources['stacker.news'])[0].match(/^app-(.+)-/)[1] // eslint-disable-line
-  : require('child_process').execSync('git rev-parse HEAD').toString().slice(0, 4)
+// const commitHash = isProd
+//   ? Object.keys(require('/opt/elasticbeanstalk/deployment/app_version_manifest.json').RuntimeSources['stacker.news'])[0].match(/^app-(.+)-/)[1] // eslint-disable-line
+//   : require('child_process').execSync('git rev-parse HEAD').toString().slice(0, 4)

 module.exports = withPlausibleProxy()({
   env: {
-    NEXT_PUBLIC_COMMIT_HASH: commitHash,
+    // NEXT_PUBLIC_COMMIT_HASH: commitHash,
     NEXT_PUBLIC_LND_CONNECT_ADDRESS: process.env.LND_CONNECT_ADDRESS,
     NEXT_PUBLIC_ASSET_PREFIX: isProd ? 'https://a.stacker.news' : ''
   },
@@ -30,7 +30,7 @@ module.exports = withPlausibleProxy()({
     scrollRestoration: true
   },
   reactStrictMode: true,
-  generateBuildId: isProd ? async () => commitHash : undefined,
+  // generateBuildId: isProd ? async () => commitHash : undefined,
   // Use the CDN in production and localhost for development.
   assetPrefix: isProd ? 'https://a.stacker.news' : undefined,
   crossOrigin: isProd ? 'anonymous' : undefined,

I have been running this patch for so long, I totally forgot this is an issue, lol. We should find a better solution to this, especially now where more people start contributing.

edit: Just setting commitHash to undefined as @huumn mentioned would also work. Seems like I used the complicated workaround, haha

[huumn]

I'm not sure why this happens for you all though. What are your perms ek?

[ekzyis]

Everything in the cloned repository is owned by me, ekzyis.

However, I think this is unrelated to our file permissions.
It's more related to the file permissions inside the docker container: Everything which was not mounted by us is owned by root.

As mentioned here, since CVE-2022-24765, git now does the following:

Git now examines parent directories to determine whether the same user owns both directories.

This means it now checks that the source folder inside the container is owned by the same user as the parent folder. But this is not the case, the source folder is owned by me but / (since we mount the source code at /app) is owned by root. The linked article has a solution involving changing a devcontainer.json file but I am not sure if it's worth to change any deep docker configuration just for this.

I also got used to my git rebase --onto master local-dev <branch> approach, lol

edit: Or it's because the files inside /app are owned by us but the folder itself is owned by root? Since I am not sure which parent directory is meant. Could be the parent of the source code or parent of .git

[huumn]

Probably the parent of .git.

I must be on an older version of git where this check doesn't happen.

[huumn]

I just pushed a change that catches this git error if it happens.

You shouldn't need to monkey patch around this anymore.

[brihuang99]

I just pushed a change that catches this git error if it happens.

You shouldn't need to monkey patch around this anymore.

Awesome thanks a ton! Will try it again soon