🐛 Bug Report: Techdocs iFrame hook isSafe does not support relative paths

[Coderrob]

📜 Description

The current implementation of the isSafe function assumes all src attributes in iframe elements contain absolute URLs because the full hostname is needed to compare against the allowed iframes configuration array.

Regex is not supported, wildcards are not supported, and we're stuck with having to be a full absolute path.

However, when an iframe has a relative path (e.g., ./something.html), the URL constructor throws an error, leading the function to return false, marking the iframe as unsafe regardless of its validity within the host environment.

Note: The Techdocs specific iframe allowed sources was not intuitive. Expectation was CSP framesources would be the "safety" check needed or the default allowed list. That'd require wildcard expression support for the allowed frame sources, which I can add as a new feature request if this morphs into something larger.

This creates issues when working in environments where relative paths are valid or required.

https://github.com/backstage/backstage/blob/master/plugins/techdocs/src/reader/transformers/html/hooks/iframes.ts#L30

👍 Expected behavior

Links, images, and other elements that link to internal or external content have paths re-written in the addBaseUrl function a few directory above in the html folder.

backstage/plugins/techdocs/src/reader/transformers/addBaseUrl.ts

Line 86 in a4d9d57

await Promise.all([

Expectation is to be able to support a structure such as:

simple-docs/
├── catalog-info.yaml
├── mkdocs.yml
└── docs/
    ├── index.md
    └── emails/
        └── examples/
            └── html_template.html

Using the npx @techdocs/cli generate and npx @techdocs/cli serve show the Docker container has the html_template.html in the correct folder location.

Not supporting relative iFrame source paths blocks being able to render required assets in Backstage techdocs.

👎 Actual Behavior with Screenshots

Functionally working in mkdocs and iframe works with relative paths.

👟 Reproduction steps

simple-docs.zip

📃 Provide the context for the Bug.

We have some services that publish HTML templates of the content they offer or create for review by the developers to ensure it meets their requirements. Specifically with HTML email templates.

Being able to embed relative path content to the HTML templates is needed but not currently supported by Backstage due to issue outlined above.

🖥️ Your Environment

No response

👀 Have you spent some time to check if this bug has been raised before?

• I checked and didn't find similar issue

🏢 Have you read the Code of Conduct?

• I have read the Code of Conduct

Are you willing to submit PR?

Yes I am willing to submit a PR!

[Coderrob]

I assume that it's more nuanced than simply checking whether the fs.isAbsolute / startsWith('.') on the source link and allowing it due to the routing / base url re-writing.

If it isn't (haven't had cycles to test local package change / import) I had Mr GPT do a quick refactoring on the TS to support relative paths. But you know how the results go lol ... no expectation of functioning without the existing test checks

/**
 * Checks whether a node is an iframe.
 * @param node - Can be any element.
 * @returns true if the node is an iframe.
 */
const isIframe = (node: Element): boolean => node.nodeName === 'IFRAME';

/**
 * Attempts to construct a URL, using the document base URI if src is relative.
 * @param src - The source URL or path.
 * @returns Parsed URL or null if invalid.
 */
const parseUrl = (src: string): URL | null => {
  try {
    return new URL(src, document.baseURI);
  } catch {
    return null;
  }
};

/**
 * Checks if an iframe's src is within allowed hosts.
 * @param src - The src attribute of the iframe.
 * @param hosts - Allowed host names.
 * @returns true if the iframe is safe.
 */
const isSrcAllowed = (src: string, hosts: string[]): boolean => {
  const url = parseUrl(src);
  return url ? hosts.includes(url.host) : false;
};

/**
 * Removes unsafe iframes based on allowed hosts.
 * @param hosts - List of allowed hosts.
 * @returns A function that removes unsafe iframes.
 */
export const removeUnsafeIframes = (hosts: string[]) => (node: Element): Element => {
  if (isIframe(node) && !isSrcAllowed(node.getAttribute('src') || '', hosts)) {
    node.remove();
  }
  return node;
};

[CiscoRob]

^^ techdocs/cli serve

[jescalada]

Hi @CiscoRob @Coderrob, I found out a way to implement this by modifying the mkdocs configuration (mkdocs.yml):

---
site_name: Simple Docs
nav:
  - 'Home': './index.md'
docs_dir: docs
repo_url: https://localhost:3000/
edit_uri_template: edit/main/data/docs/simple-docs/docs/{path}
plugins:
  - techdocs-core
markdown_extensions:
  - md_in_html
extra_templates:
  - emails/examples/html_template.html

This will host the file on the backend, in this case http://localhost:7007/api/techdocs/static/docs/default/resource/simple-docs/emails/examples/html_template.html. I implemented some changes to the iframes.ts in order to support parsing the relative URLs in the following PR: #27923

The issue is that the X-Frame-Options header needs to be changed in order to allow iframes from localhost:7007, which I suppose is a different host from localhost:3000. Setting that header manually would be a simple workaround rather than reimplementing the techdocs to upload the template to localhost (unless there's an easier way I'm not aware of!).

Let me know your thoughts 🙂

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.