Script diagnostics#7320
Conversation
|
Great job! No new security vulnerabilities introduced in this pull requestUse @Checkmarx to interact with Checkmarx PR Assistant. |
bee495d to
2a31eff
Compare
Signed-off-by: kingthorin <kingthorin@users.noreply.github.com>
2a31eff to
2cda25c
Compare
There was a problem hiding this comment.
Pull request overview
This pull request introduces persistence and reporting of automation script execution failures in the Scripts add-on, exposing that data to the Reports add-on’s “traditional-json-plus” template via an optional section.
Changes:
- Add JDO/Flyway-backed session DB schema and entities to persist script automation failures (with a placeholder screenshot table + FK migration).
- Record failures from automation execution paths and provide report-time querying + a Scripts report extension that supplies rows to
ReportData. - Extend the “traditional-json-plus” report template, i18n, help docs, and unit tests to cover the new
scriptAutomationFailuresoutput.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| addOns/scripts/src/test/java/org/zaproxy/zap/extension/scripts/internal/db/TableJdoUnitTest.java | Adds unit tests for scripts add-on TableJdo lifecycle/PMF behavior. |
| addOns/scripts/src/test/java/org/zaproxy/zap/extension/scripts/internal/db/ScriptFailureRecorderUnitTest.java | Tests that recorder is a no-op on blank messages. |
| addOns/scripts/src/test/java/org/zaproxy/zap/extension/scripts/automation/actions/RunScriptActionUnitTest.java | Adds unit tests around failure recording / runtime chain failure behavior. |
| addOns/scripts/src/test/java/org/zaproxy/zap/extension/scripts/automation/ScriptAutomationFailureRecordsUnitTest.java | Tests bridging from job parameters to persisted failure entity fields. |
| addOns/scripts/src/main/resources/org/zaproxy/zap/extension/scripts/resources/Messages.properties | Adds i18n strings for the new Scripts report extension. |
| addOns/scripts/src/main/resources/db/migration/V2__Scripts_failure_screenshot_fk.sql | Adds FK migration for future screenshot rows. |
| addOns/scripts/src/main/resources/db/migration/V1__Create_scripts_failure_tables.sql | Adds initial schema for failure + screenshot tables. |
| addOns/scripts/src/main/resources/META-INF/persistence.xml | Declares persistence unit and entity classes for scripts add-on JDO enhancement. |
| addOns/scripts/src/main/java/org/zaproxy/zap/extension/scripts/report/ScriptAutomationFailureRow.java | Adds report-facing row DTO for templates. |
| addOns/scripts/src/main/java/org/zaproxy/zap/extension/scripts/report/ExtensionScriptsReport.java | Adds report extension to supply script failure rows to Report Generation add-on. |
| addOns/scripts/src/main/java/org/zaproxy/zap/extension/scripts/internal/db/TableJdo.java | Adds scripts add-on TableJdo to run Flyway + create PMF for session DB. |
| addOns/scripts/src/main/java/org/zaproxy/zap/extension/scripts/internal/db/ScriptFailureScreenshot.java | Adds JDO entity mapping for future screenshot persistence. |
| addOns/scripts/src/main/java/org/zaproxy/zap/extension/scripts/internal/db/ScriptFailureReportQuery.java | Adds query to load persisted failures and map them to report rows. |
| addOns/scripts/src/main/java/org/zaproxy/zap/extension/scripts/internal/db/ScriptFailureRecorder.java | Adds failure recorder utility to persist failures without propagating errors. |
| addOns/scripts/src/main/java/org/zaproxy/zap/extension/scripts/internal/db/ScriptFailure.java | Adds JDO entity mapping for script failures. |
| addOns/scripts/src/main/java/org/zaproxy/zap/extension/scripts/automation/actions/ScriptAction.java | Centralizes progress error reporting to avoid persisting plan/parameter errors. |
| addOns/scripts/src/main/java/org/zaproxy/zap/extension/scripts/automation/actions/RunScriptAction.java | Records execution failures to session DB and improves chained-script error reporting. |
| addOns/scripts/src/main/java/org/zaproxy/zap/extension/scripts/automation/ScriptJob.java | Minor refactor to capture error message string before reporting. |
| addOns/scripts/src/main/java/org/zaproxy/zap/extension/scripts/automation/ScriptAutomationFailureRecords.java | Adds bridge helper from automation params to persistence recorder. |
| addOns/scripts/src/main/java/org/zaproxy/zap/extension/scripts/automation/ExtensionScriptAutomation.java | Initializes/tears down scripts TableJdo and records script errors for running plans. |
| addOns/scripts/scripts.gradle.kts | Enables JDO enhancement for scripts add-on and adds required add-on deps. |
| addOns/scripts/CHANGELOG.md | Documents new optional report output for script automation failures. |
| addOns/reports/src/test/java/org/zaproxy/addon/reports/ReportTestUtils.java | Adds helper to generate reports containing script automation failure objects. |
| addOns/reports/src/test/java/org/zaproxy/addon/reports/ExtensionReportsJsonUnitTest.java | Adds JSON assertions for new scriptAutomationFailures output and section toggling. |
| addOns/reports/src/main/zapHomeFiles/reports/traditional-json-plus/template.yaml | Adds scriptautomationfailures section to template section list. |
| addOns/reports/src/main/zapHomeFiles/reports/traditional-json-plus/report.json | Adds templated JSON output for scriptAutomationFailures array. |
| addOns/reports/src/main/zapHomeFiles/reports/traditional-json-plus/Messages.properties | Adds i18n label for the new template section. |
| addOns/reports/src/main/javahelp/org/zaproxy/addon/reports/resources/help/contents/report-traditional-json-plus.html | Documents the new template section and sensitivity/size considerations. |
| addOns/reports/reports.gradle.kts | Adds scripts add-on as a test dependency for report tests. |
| addOns/reports/CHANGELOG.md | Documents the new optional scriptAutomationFailures section in traditional-json-plus. |
| addOns/client/src/test/java/org/zaproxy/addon/client/internal/db/TableJdoUnitTest.java | Updates cleanup to reset static PMF between tests (pattern reused in scripts tests). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Properties jdoProperties = new Properties(); | ||
| jdoProperties.setProperty( | ||
| Constants.PROPERTY_PERSISTENCE_MANAGER_FACTORY_CLASS, | ||
| JDOPersistenceManagerFactory.class.getName()); | ||
| jdoProperties.setProperty(Constants.PROPERTY_PERSISTENCE_UNIT_NAME, "scripts"); | ||
| jdoProperties.setProperty(Constants.PROPERTY_CONNECTION_URL, dbUrl); | ||
| jdoProperties.setProperty(Constants.PROPERTY_CONNECTION_USER_NAME, user); | ||
| jdoProperties.setProperty(Constants.PROPERTY_CONNECTION_PASSWORD, password); | ||
|
|
||
| jdoProperties.put(PropertyNames.PROPERTY_CLASSLOADER_PRIMARY, classLoader); | ||
| jdoProperties.put( | ||
| RDBMSPropertyNames.PROPERTY_RDBMS_STRING_LENGTH_EXCEEDED_ACTION, "TRUNCATE"); | ||
|
|
||
| pmf = JDOHelper.getPersistenceManagerFactory(jdoProperties, classLoader); | ||
| } |
There was a problem hiding this comment.
TableJdo PMF creation differs from the established add-on pattern (e.g. client/authhelper) which calls JDOHelper.getPersistenceManagerFactory(props, "<addonId>", classLoader) to select the persistence unit and keep PMFs isolated by name. Here the code sets PROPERTY_PERSISTENCE_UNIT_NAME and calls the 2-arg overload instead; please align with the other TableJdo implementations by passing the persistence unit name as the second argument to getPersistenceManagerFactory (and the extra PMF-class/unit-name properties can likely be dropped).
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Rick M <kingthorin@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Rick M <kingthorin@users.noreply.github.com>

Overview
Related Issues