Skip to content

fix: Decrypt encrypted front component variables - #23494

Merged
prastoin merged 5 commits into
twentyhq:mainfrom
remihuigen:fix/encrypted-app-vars
Jul 30, 2026
Merged

fix: Decrypt encrypted front component variables#23494
prastoin merged 5 commits into
twentyhq:mainfrom
remihuigen:fix/encrypted-app-vars

Conversation

@remihuigen

@remihuigen remihuigen commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #23492

Fixes front-component application variables returning their encrypted at-rest value instead of their configured plaintext value.

Non-secret application variables (isSecret: false) are now decrypted server-side before being injected into the front-component environment. Secret variables remain excluded and are never decrypted or exposed to the browser.

Root cause

The front-component resolver filtered secret application variables correctly, but forwarded the cached encryptedValue directly. As a result, getApplicationVariable() returned an enc:v2:... envelope rather than the configured value.

Changes

  • Decrypt recognized versioned envelopes for non-secret application variables.
  • Preserve empty and legacy/plain values unchanged for backwards compatibility.
  • Add SecretEncryptionModule to the front-component module.
  • Add coverage for:
    • decrypting public variables;
    • retaining plaintext compatibility;
    • excluding secret variables without attempting decryption.

Review in cubic

Copilot AI review requested due to automatic review settings July 29, 2026 11:39
@twenty-ci-bot-public

Copy link
Copy Markdown

👋 Thanks for contributing to Twenty! We're excited to have you on board.

Your PR has been set to draft while you work on it. Once you're done, mark it as Ready for review and our automated checks will run.

By submitting your Pull Request, you acknowledge that you agree with the terms of our Contributor License Agreement.

@greptile-apps

greptile-apps Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Fixes front-component application-variable injection by:

  • Centralizing cached variable lookup and decryption in ApplicationVariableEntityService.
  • Exposing only decrypted non-secret variables to front components.
  • Reusing the centralized environment builder for logic-function execution.
  • Adding unit and integration coverage for decryption and secret exclusion.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
packages/twenty-server/src/engine/core-modules/application/application-variable/application-variable.service.ts Adds centralized cached lookup and environment-record construction for decrypted application variables.
packages/twenty-server/src/engine/metadata-modules/front-component/front-component.resolver.ts Replaces direct cached-value forwarding with the non-secret decrypted environment builder.
packages/twenty-server/src/engine/core-modules/application/application-variable/tests/application-variable.service.spec.ts Consolidates variable decryption tests and now verifies exactly one decryption call for the non-secret path.
packages/twenty-server/src/engine/core-modules/logic-function/logic-function-executor/logic-function-executor.service.ts Delegates logic-function application-variable environment construction to the centralized service.
packages/twenty-server/test/integration/metadata/suites/front-component/successful-front-component-application-variables.integration-spec.ts Adds end-to-end coverage that public variables are encrypted at rest, decrypted for front components, and secret variables remain excluded.

Reviews (3): Last reviewed commit: "test(server): integ test front comp appl..." | Re-trigger Greptile

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes front-component application variables (isSecret: false) being injected into the front-component runtime as their encrypted at-rest envelope (enc:v2:...) instead of plaintext, by decrypting eligible values server-side while continuing to omit secret variables.

Changes:

  • Pass SecretEncryptionService into the front-component variable sanitization utility and decrypt non-secret encrypted values before serialization.
  • Wire SecretEncryptionModule into the front-component Nest module so the resolver can inject the encryption service.
  • Extend unit tests to cover decryption behavior and ensure secret variables are excluded without attempting decryption.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
packages/twenty-server/src/engine/metadata-modules/front-component/utils/strip-secret-from-application-variables.ts Adds server-side decryption for non-secret application variable values before injecting them into front-component env.
packages/twenty-server/src/engine/metadata-modules/front-component/utils/tests/strip-secret-from-application-variables.spec.ts Updates and adds tests covering decryption and secret-variable exclusion behavior.
packages/twenty-server/src/engine/metadata-modules/front-component/front-component.resolver.ts Injects SecretEncryptionService and supplies it to the stripping/decryption utility.
packages/twenty-server/src/engine/metadata-modules/front-component/front-component.module.ts Imports SecretEncryptionModule to make encryption services available in the front-component module.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 145 to 165
it('should decrypt non-secret encrypted variables', () => {
const encryptedValue = 'enc:v2:key-id:payload' as EncryptedString;

decryptVersionedOrThrow.mockReturnValue('pk.mapbox-token');

expect(
stripSecretFromApplicationVariables(
[
makeFlatVariable({
key: 'MAPBOX_PUBLIC_ACCESS_TOKEN',
value: encryptedValue,
}),
],
secretEncryptionService,
),
).toEqual({ MAPBOX_PUBLIC_ACCESS_TOKEN: 'pk.mapbox-token' });
expect(decryptVersionedOrThrow).toHaveBeenCalledWith(encryptedValue, {
workspaceId: '00000000-0000-0000-0000-000000000000',
});
});
});
Comment on lines +20 to +24
isNonEmptyString(value) && isEncryptedString(value)
? secretEncryptionService.decryptVersionedOrThrow(value, {
workspaceId: flatApplicationVariable.workspaceId,
})
: value;
@martmull

Copy link
Copy Markdown
Contributor

We don't expose secret variables in front components for security reasons, you can access them via logic-functions

https://docs.twenty.com/developers/extend/apps/layout/front-components#application-variables

image

@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor
Fails
🚫

node failed.

Log

Details
�[31mError: �[39m SyntaxError: Unexpected token '<', "<!DOCTYPE "... is not valid JSON
    at JSON.parse (<anonymous>)
�[90m    at parseJSONFromBytes (node:internal/deps/undici/undici:4387:19)�[39m
�[90m    at successSteps (node:internal/deps/undici/undici:7041:27)�[39m
�[90m    at readAllBytes (node:internal/deps/undici/undici:5958:13)�[39m
�[90m    at process.processTicksAndRejections (node:internal/process/task_queues:104:5)�[39m
danger-results://tmp/danger-results-0f65ddb3.json

Generated by 🚫 dangerJS against fab152f

@prastoin prastoin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there thanks for reporting, double checking

Copilot AI review requested due to automatic review settings July 30, 2026 15:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

@prastoin prastoin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for reporting this, that's unfortunate
Refactored few things
Will be released tomorrow along 2.26 ( twenty engine )

@prastoin

Copy link
Copy Markdown
Member

@greptileai review

@twenty-ci-bot-public

twenty-ci-bot-public Bot commented Jul 30, 2026

Copy link
Copy Markdown

🔍 Automated Pre-Review

No issues detected - This PR is ready for human review.


🧭 External PR Triage Review

Looks good — a maintainer can pick this up.

Checks

  • CI: pending
  • 🔗 Linked issue: Fixes #23492

Detailed findings (duplicate candidates, standards notes, summary) are in the workflow run logs.


View details

Automated pre-review — human approval still required.

@remihuigen

remihuigen commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

@prastoin thanks! (I tried to keep the surface of my PR minimal because I'm not too familiar with the code and its conventions.) The refactor looks good.

It's kind of baffling that no one—neither users nor maintainers—noticed this feature shipped in a broken state back in v2.10.

Anyway, glad it's included in tomorrow's release.

@prastoin

prastoin commented Jul 30, 2026

Copy link
Copy Markdown
Member

@remihuigen no worries ! reporting is way more than enough

It's kind of baffling that no one—neither users nor maintainers—noticed this feature shipped in a broken state back in v2.10.

You took the words right out of my mouth
Might be a regression introduced more recently following the database encryption consistency refactor

@Weiko Weiko left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rename the "dangerous" one buildEnvRecordIncludingSecrets or something like that now that you have buildEnvRecord / buildNonSecretEnvRecord? (you can probably find a better name)

Copilot AI review requested due to automatic review settings July 30, 2026 16:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

Copilot AI review requested due to automatic review settings July 30, 2026 16:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

@prastoin
prastoin enabled auto-merge July 30, 2026 16:39
@prastoin
prastoin added this pull request to the merge queue Jul 30, 2026
Merged via the queue into twentyhq:main with commit 830404b Jul 30, 2026
122 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

getApplicationVariable() returns encrypted values in front components

5 participants