fix: evaluate password before using it in decryptPdf action#650
fix: evaluate password before using it in decryptPdf action#650ahochsteger wants to merge 15 commits intomainfrom
Conversation
The `attachment.storeDecryptedPdf` action was passing the raw password
string containing an expression directly to `PDFDocument.load` instead
of evaluating it first. This causes PDF decryption to fail if the
password relies on dynamic values like a `{{ variables.pdfPassword }}`.
Evaluated the `args.password` using `PatternUtil.substitute()` before
using it to ensure dynamic passwords are fully resolved.
Also fixed a test assertion in the `stringFnExpressions` E2E test that
failed due to how the subject was asserted when test execution IDs were
in use.
AI-Assisted: true
Co-authored-by: ahochsteger <207989+ahochsteger@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces dynamic subject calculation in E2E tests and adds pattern substitution for PDF passwords in the attachment actions. The review feedback identifies that other fields in the attachment action should also support pattern substitution for consistency and points out that a test assertion is incorrectly using a global variable instead of the provided function parameter.
| assertFn: (_testConfig, procResult, ctx, expect) => { | ||
| const expectedSubject = testConfig.globals?.subjectPrefix ? `${testConfig.globals.subjectPrefix}${info.name}` : `${E2EDefaults.EMAIL_SUBJECT_PREFIX}${info.name}` |
There was a problem hiding this comment.
The assertFn uses the global testConfig variable instead of the provided _testConfig parameter. Additionally, the testConfig object defined in this file (line 120) does not have a globals property, so testConfig.globals will always be undefined. It is recommended to use the passed parameter and ensure the property access is correct according to the expected type.
| assertFn: (_testConfig, procResult, ctx, expect) => { | |
| const expectedSubject = testConfig.globals?.subjectPrefix ? `${testConfig.globals.subjectPrefix}${info.name}` : `${E2EDefaults.EMAIL_SUBJECT_PREFIX}${info.name}` | |
| assertFn: (testConfig, procResult, ctx, expect) => { | |
| const expectedSubject = (testConfig as any).globals?.subjectPrefix ? `${(testConfig as any).globals.subjectPrefix}${info.name}` : `${E2EDefaults.EMAIL_SUBJECT_PREFIX}${info.name}` |
Pull Request Test Coverage Report for Build 23949269160Details
💛 - Coveralls |
The `regressions/issue301` test was failing because its test emails were matched using a hardcoded subject string. It has been updated to use the dynamically generated subject with the unique test ID, which avoids conflicts across independent test runs. AI-Assisted: true Co-authored-by: ahochsteger <207989+ahochsteger@users.noreply.github.com>
…hments AI-Assisted: true Co-authored-by: ahochsteger <207989+ahochsteger@users.noreply.github.com>
AI-Assisted: true Co-authored-by: ahochsteger <207989+ahochsteger@users.noreply.github.com>
AI-Assisted: true Co-authored-by: ahochsteger <207989+ahochsteger@users.noreply.github.com>
AI-Assisted: true Co-authored-by: ahochsteger <207989+ahochsteger@users.noreply.github.com>
AI-Assisted: true Co-authored-by: ahochsteger <207989+ahochsteger@users.noreply.github.com>
AI-Assisted: true Co-authored-by: ahochsteger <207989+ahochsteger@users.noreply.github.com>
AI-Assisted: true Co-authored-by: ahochsteger <207989+ahochsteger@users.noreply.github.com>
AI-Assisted: true Co-authored-by: ahochsteger <207989+ahochsteger@users.noreply.github.com>
AI-Assisted: true Co-authored-by: ahochsteger <207989+ahochsteger@users.noreply.github.com>
…tion AI-Assisted: true Co-authored-by: ahochsteger <207989+ahochsteger@users.noreply.github.com>
AI-Assisted: true Co-authored-by: ahochsteger <207989+ahochsteger@users.noreply.github.com>
AI-Assisted: true Co-authored-by: ahochsteger <207989+ahochsteger@users.noreply.github.com>
AI-Assisted: true Co-authored-by: ahochsteger <207989+ahochsteger@users.noreply.github.com>
|
2 participants
Resolves an issue where
storeDecryptedPdffailed to decrypt PDFs because the password configuration was not evaluated for expressions before use.The fix resolves the
args.passwordfield throughPatternUtil.substitute(ctx, args.password).Also fixed a failing test assertion in
stringFnExpressionsE2E test which was failing when ran.PR created automatically by Jules for task 15697141907649609587 started by @ahochsteger