Potential issue with PUBLIC_ORG and PRIVATE_ORG logic

[jeffabailey]

To avoid a potentially confusing PR out of the blue, I'm explaining my situation:

It appears the PRIVATE_ORG environment variable isn't read by the app.

I've spun up a deployment of the app to a lambda hosted container.

After these changes, I've successfully installed the app in a public and private org, and successfully copied a public fork to my private org.

fork --> public org --> + Create Mirror --> (set repo name) --> private org (mirror success)

I had to apply these patches to get it to work.

---

Not needed to fix the issue, but useful to see what's configured in the Lambda.

--- a/src/server/config/controller.ts
+++ b/src/server/config/controller.ts
@@ -14,7 +14,10 @@ export const getConfigHandler = async ({
 
     const config = await getConfig(input.orgId)
 
-    configApiLogger.debug('Fetched config', config)
+    configApiLogger.info('Config resolved', {
+      publicOrg: config.publicOrg,
+      privateOrg: config.privateOrg,
+    })
 
     return config
   } catch (error) {

---

Looks like where the privateOrg data isn't picked up when PRIVATE_ORG is defined.

--- a/src/server/repos/controller.ts
+++ b/src/server/repos/controller.ts
@@ -49,34 +49,28 @@ export const createMirrorHandler = async ({
     const privateInstallationId = octokitData.private.installationId
     const privateAccessToken = octokitData.private.accessToken
 
-    const orgData = await contributionOctokit.rest.orgs.get({
-      org: publicOrg,
-    })
-
-    await contributionOctokit.rest.repos
+    // Check if the mirror repo already exists in the private org (not the public fork org)
+    await privateOctokit.rest.repos
       .get({
-        owner: orgData.data.login,
+        owner: privateOrg,
         repo: input.newRepoName,
       })
       .then((res) => {
-        // if we get a response, then we know the repo exists so we throw an error
         if (res.status === 200) {
           reposApiLogger.info(
-            `Repo ${orgData.data.login}/${input.newRepoName} already exists`,
+            `Repo ${privateOrg}/${input.newRepoName} already exists`,
           )
 
           throw new Error(
-            `Repo ${orgData.data.login}/${input.newRepoName} already exists`,
+            `Repo ${privateOrg}/${input.newRepoName} already exists`,
           )
         }
       })
       .catch((error) => {
-        // catch and rethrow the error if the repo already exists
         if ((error as Error).message.includes('already exists')) {
           throw error
         }
 
-        // if there is a real error, then we log it and throw it
         if (!(error as Error).message.includes('Not Found')) {
           reposApiLogger.error('Not found', { error })
           throw error

---

My AI friend explained why this change is required:

I asked it if the Lamba is the problem, and not the Private Mirror App, and it responded:

Yes: the Lambda is not the problem. Terraform sets PUBLIC_ORG and PRIVATE_ORG in the Lambda environment.variables block in main.tf (lines 169–170), and Lambda passes them at runtime. The issue is when the app reads them.
Why process.env['PUBLIC_ORG'] “fixes” it (and it’s not about Lambda):
The image is built in Docker with npm run build and no PUBLIC_ORG/PRIVATE_ORG in the build environment. Next.js/the bundler does static substitution: process.env.PUBLIC_ORG is replaced at build time with whatever value existed then (here, undefined). So the running code never does a real env lookup; it’s already compiled to that build-time value. Lambda’s env is correct, but the bundle doesn’t read it. Using bracket notation process.env['PUBLIC_ORG'] is a dynamic property access, so the bundler typically doesn’t substitute it and the lookup happens at runtime, where Lambda’s env is visible. So the fix is “read at runtime,” not “Lambda was wrong.”

--- a/src/bot/config.ts
+++ b/src/bot/config.ts
@@ -30,7 +30,7 @@ export const getGitHubConfig = async (orgId: string) => {
 }
 
 export const getEnvConfig = () => {
-  if (!process.env.PUBLIC_ORG) {
+  if (!process.env['PUBLIC_ORG']) {
     return null
   }
 
@@ -39,12 +39,12 @@ export const getEnvConfig = () => {
   )
 
   const config = {
-    publicOrg: process.env.PUBLIC_ORG,
-    privateOrg: process.env.PUBLIC_ORG,
+    publicOrg: process.env['PUBLIC_ORG'],
+    privateOrg: process.env['PUBLIC_ORG'],
   } as pmaConfig
 
-  if (process.env.PRIVATE_ORG) {
-    config.privateOrg = process.env.PRIVATE_ORG
+  if (process.env['PRIVATE_ORG']) {
+    config.privateOrg = process.env['PRIVATE_ORG']
   }
   return config
 }

---

Reads line 166, which makes the PRIVATE_ORG show up as a mirroring destination:

const orgLogin = getConfigData?.privateOrg ?? orgData?.data?.login

--- a/src/app/[organizationId]/forks/[forkId]/page.tsx
+++ b/src/app/[organizationId]/forks/[forkId]/page.tsx
@@ -427,7 +427,7 @@ const Fork = () => {
           </Blankslate>
         </Box>
         <CreateMirrorDialog
-          orgLogin={orgData?.data?.login as string}
+          orgLogin={orgLogin as string}
           forkParentName={forkData?.data?.parent?.name as string}
           forkParentOwnerLogin={forkData?.data?.parent?.owner.login as string}
           closeDialog={closeCreateDialog}

Is there something I'm missing, or is there an issue with the private vs. public mirroring logic?

[wrslatz]

I'm not 100% sure the AI explanation there about build-time substitution makes sense, based on https://nextjs.org/docs/pages/guides/environment-variables#bundling-environment-variables-for-the-browser, unless you're triggering that scenario accidentally at build-time? Dot vs bracket notation on process.env should not make a difference here.

Have you verified the environment variable is being properly passed to the container at runtime? And have you verified the Lambda artifact does not contain the substituted value from build-time but still references process.env...?

[jeffabailey]

@wrslatz thank you for the reply!

I went back to the drawing board.

Nuked the entire stack.

Deployed it using this container.

ghcr.io/github-community-projects/private-mirrors:latest

It's working as expected.