Skip to content

Verify the TLS certificate when sending media for transcoding - #2241

Merged
subodhr258 merged 2 commits into
mainfrom
fix/transcoder-verify-ssl
Oct 26, 2026
Merged

subodhr258 merged 2 commits into
mainfrom
fix/transcoder-verify-ssl

Conversation

@subodhr258

Copy link
Copy Markdown
Collaborator

What changes

The two requests that send a file to the GoDAM API for transcoding turned TLS certificate verification off ('sslverify' => false):

  • admin/class-rtgodam-transcoder-handler.php: media uploads and retranscodes.
  • inc/helpers/custom-functions.php: form recordings (Gravity Forms, Fluent Forms, Ninja Forms, SureForms, WPForms, Everest Forms).

Both now verify the certificate, like every other request the plugin makes to the same API. The setting dates from 2016, when the API address was plain http.

A new filter, godam_transcoder_verify_ssl (default true), lets a local or staging API with a self-signed certificate turn verification off, the same way godam_frappe_dispatch_verify_ssl works for the Frappe installer:

add_filter( 'godam_transcoder_verify_ssl', '__return_false' );

Will anything break?

GET /api/method/ping on each API host with certificate verification on:

API host WordPress's HTTP library, latest CA bundle Same, WordPress 6.5 CA bundle (oldest supported) A real local site: wp_remote_get(), sslverify true (WordPress 7.1.3, PHP 8.2)
app.godam.io (production) 200 200 200
app-godam.rt.gw (staging) 200 200 200
app-godam-preprod.rt.gw (preprod) 200 200 200
  • All three hosts use Google Trust Services certificates, and those roots are in WordPress's own CA bundle (the bundle WordPress uses when sslverify is true), from 6.5 onwards at least.
  • An http:// API address (some local setups) is unaffected: sslverify only applies to https.
  • A local API with a self-signed certificate needs the filter above.

Testing

  • php -l and phpcs on both files: clean.
  • No unit test: the change is one request option, and the HTTPS check above is what matters.

QA:

  1. On a site connected to GoDAM, upload a video: it is sent for transcoding and transcodes as before.
  2. Submit a form recording with any form plugin that has the GoDAM recorder field: it transcodes as before.
  3. Repeat 1 on a site with RTGODAM_API_BASE set to preprod.

Release ticket: rtCamp/godam-plugin-wp#68

@subodhr258 subodhr258 self-assigned this Oct 22, 2026
@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

🔍 WordPress Plugin Check Report

⚠️ Status: Passed with warnings

📊 Report

🎯 Total Issues ❌ Errors ⚠️ Warnings
13 0 13

⚠️ Warnings (13)

📁 readme.txt (2 warnings)
📍 Line 🔖 Check 💬 Message
0 mismatched_plugin_name Plugin name "GoDAM - Organize WordPress Media Library & File Manager with Unlimited Folders for Images, Videos & more" is different from the name declared in plugin header "GoDAM".
0 trademarked_term The plugin name includes a restricted term. Your chosen plugin name - "GoDAM - Organize WordPress Media Library & File Manager with Unlimited Folders for Images, Videos & more" - contains the restricted term "wordpress" which cannot be used at all in your plugin name.
📁 assets/build/css/main.css (1 warning)
📍 Line 🔖 Check 💬 Message
0 EnqueuedStylesScope This style is being loaded in all contexts.
📁 assets/src/libs/analytics.min.js (5 warnings)
📍 Line 🔖 Check 💬 Message
0 EnqueuedScriptsScope This script is being loaded in all frontend contexts.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880 (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/2026/10/26/hello-world/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/sample-page/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/demo-attachment-post/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
📁 assets/build/js/main.min.js (5 warnings)
📍 Line 🔖 Check 💬 Message
0 EnqueuedScriptsScope This script is being loaded in all frontend contexts.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880 (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/2026/10/26/hello-world/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/sample-page/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/demo-attachment-post/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.

🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check

The two requests that send a file to the GoDAM API for transcoding (media
uploads and form recordings) turned certificate verification off. The
setting dates from when the API address was plain http; every other request
to the same API already verifies the certificate.

Both now verify it. A new filter, godam_transcoder_verify_ssl (default
true), lets a local or staging API with a self-signed certificate turn it
off, like the existing godam_frappe_dispatch_verify_ssl filter.
@subodhr258
subodhr258 force-pushed the fix/transcoder-verify-ssl branch from b2a7ee5 to 5607f1e Compare October 23, 2026 06:21
@subodhr258
subodhr258 changed the base branch from develop to main October 23, 2026 06:21
@subodhr258

Copy link
Copy Markdown
Collaborator Author

Tested together with the other five 2.5.1 PRs on a hosted test site connected to production GoDAM Central. Full table and all recordings: https://github.com/rtCamp/godam-plugin-wp/issues/68 (recordings are in the rtCamp Test media library; sign in to watch).

Tested commit b2a7ee54c; the current head 5607f1e29 has the same code.

Every test upload sent its transcoding job to app.godam.io with certificate checks on, and all of them transcoded (70 to 104 seconds), for example T1 and T3. Central's site checks pass 6 of 6: T7.

@elifvish elifvish left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No issues found.

Verification

  • Head tested: 5607f1e297. PHPUnit on this PR's head: 613/613 OK.
  • All seven 2.5.1 PRs merged onto main (no conflicts): PHPUnit 915/915, Jest 690/690, build:prod OK.
  • Not covered: An end-to-end transcode: Central cannot reach a localhost site, so uploads fail before sending (known local limitation). The hosted-site run covers it.

Screenshots and a recording of the local run are available on request.

@KMchaudhary

Copy link
Copy Markdown
Collaborator

Review: ✅ Approvable — clean security fix

Reviewed at high effort. The change does exactly what it should and nothing more:

  • Both transcoding send points (class-rtgodam-transcoder-handler.php and custom-functions.php) now set sslverify from apply_filters( 'godam_transcoder_verify_ssl', true ), so TLS verification is on by default.
  • Swept the whole plugin for sslverify: these were the only two insecure (false) spots; the remaining occurrence (class-godam-frappe-dispatch-installer.php) already defaulted to true, and no other wp_remote_* media request disables verification.
  • The escape-hatch filter is a sensible, well-documented way to allow self-signed certs on local/staging, and it fails secure. @since 2.5.1 documented at the canonical site.

No blocking or minor issues. Good to merge. 🚀

@subodhr258
subodhr258 merged commit ebd4172 into main Oct 26, 2026
9 of 10 checks passed
@subodhr258
subodhr258 deleted the fix/transcoder-verify-ssl branch October 26, 2026 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants