This is an automated email from the ASF dual-hosted git repository.
voidmatcha pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/master by this push:
new 95b25f25fb [ZEPPELIN-6523] Sanitize href in ExternalLinkDirective
95b25f25fb is described below
commit 95b25f25fb8e19425867e04b080763adfb7ed147
Author: JangAyeon <[email protected]>
AuthorDate: Thu Oct 1 00:58:47 2026 +0900
[ZEPPELIN-6523] Sanitize href in ExternalLinkDirective
### What is this PR for?
`ExternalLinkDirective` (selector `a[href]`) declares `<at>Input() href`.
Because the directive claims the `href` input, `[href]` bindings on anchors are
delivered to the directive rather than to the DOM property, so Angular's
built-in URL sanitization (`ɵɵsanitizeUrl`) never runs. The directive then
assigned the raw value via `nativeElement.href = this.href`, which keeps
`javascript:` URLs executable on click.
One reachable path is the interpreter setting page, where a `url`-type
property value is rendered as `<a [href]="...">`
(`interpreter/item/item.component.html`). Interpreter properties are usually
admin-controlled, so the practical risk is limited, but this is still a
sanitization bypass and a defense-in-depth fix for any current or future
dynamic `[href]` binding.
This PR sanitizes the value with
`DomSanitizer.sanitize(SecurityContext.URL, ...)` before assigning it, so
unsafe URLs become `unsafe:...` just like a normal Angular `[href]` binding.
The existing `rel="noopener noreferrer"` / `target="_blank"` handling for
external links is unchanged.
### What type of PR is it?
Bug Fix
### Todos
* [x] Sanitize href in `ExternalLinkDirective`
* [x] Add unit tests
### What is the Jira issue?
[ZEPPELIN-6523](https://issues.apache.org/jira/browse/ZEPPELIN-6523)
### How should this be tested?
* Unit tests: `cd zeppelin-web-angular && npx vitest run --config
vitest.shell.config.mts src/app/share/external-links/`
### Questions:
* Does the license files need to update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
Closes #5508 from JangAyeon/ZEPPELIN-6523.
Signed-off-by: YONGJAE LEE <[email protected]>
---
.../external-links/external-link.directive.spec.ts | 90 ++++++++++++++++++++++
.../external-links/external-link.directive.ts | 12 ++-
2 files changed, 99 insertions(+), 3 deletions(-)
diff --git
a/zeppelin-web-angular/src/app/share/external-links/external-link.directive.spec.ts
b/zeppelin-web-angular/src/app/share/external-links/external-link.directive.spec.ts
new file mode 100644
index 0000000000..f63c28d432
--- /dev/null
+++
b/zeppelin-web-angular/src/app/share/external-links/external-link.directive.spec.ts
@@ -0,0 +1,90 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { Component } from '@angular/core';
+import { ComponentFixture, TestBed } from '@angular/core/testing';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+
+import { ExternalLinkDirective } from './external-link.directive';
+
+@Component({
+ standalone: false,
+ template: `
+ <a [href]="url">link</a>
+ `
+})
+class HostComponent {
+ url = '';
+}
+
+/**
+ * The directive declares `@Input() href`, so a `[href]` binding goes to the
directive
+ * instead of the anchor's DOM property and skips Angular's built-in URL
sanitization.
+ * These tests drive the directive through a real `[href]` binding to cover
that path.
+ */
+describe('ExternalLinkDirective', () => {
+ let fixture: ComponentFixture<HostComponent>;
+
+ const anchor = (): HTMLAnchorElement =>
fixture.nativeElement.querySelector('a');
+
+ const render = (url: string) => {
+ fixture.componentInstance.url = url;
+ // TestBed is zoneless by default, so flag the host dirty before
re-rendering.
+ fixture.componentRef.changeDetectorRef.markForCheck();
+ fixture.detectChanges();
+ };
+
+ beforeEach(() => {
+ // Angular logs a warning whenever it sanitizes a URL; keep the test
output clean.
+ vi.spyOn(console, 'warn').mockImplementation(() => undefined);
+
+ TestBed.configureTestingModule({
+ declarations: [HostComponent, ExternalLinkDirective]
+ });
+
+ fixture = TestBed.createComponent(HostComponent);
+ });
+
+ afterEach(() => {
+ vi.restoreAllMocks();
+ });
+
+ it('neutralizes javascript: URLs', () => {
+ render('javascript:alert(document.domain)');
+
+ expect(anchor().getAttribute('href')).toMatch(/^unsafe:/);
+ expect(anchor().protocol).not.toBe('javascript:');
+ });
+
+ it('neutralizes javascript: URLs set after an initial safe URL', () => {
+ render('https://zeppelin.apache.org/');
+ render('javascript:alert(document.domain)');
+
+ expect(anchor().getAttribute('href')).toMatch(/^unsafe:/);
+ });
+
+ it('keeps safe external URLs and opens them in a new tab', () => {
+ render('https://zeppelin.apache.org/');
+
+ expect(anchor().href).toBe('https://zeppelin.apache.org/');
+ expect(anchor().getAttribute('rel')).toBe('noopener noreferrer');
+ expect(anchor().getAttribute('target')).toBe('_blank');
+ });
+
+ it('keeps same-origin URLs without rel/target', () => {
+ render(`${location.origin}/#/notebook/abc`);
+
+ expect(anchor().href).toBe(`${location.origin}/#/notebook/abc`);
+ expect(anchor().hasAttribute('rel')).toBe(false);
+ expect(anchor().hasAttribute('target')).toBe(false);
+ });
+});
diff --git
a/zeppelin-web-angular/src/app/share/external-links/external-link.directive.ts
b/zeppelin-web-angular/src/app/share/external-links/external-link.directive.ts
index f1a5da244f..59b209e38d 100644
---
a/zeppelin-web-angular/src/app/share/external-links/external-link.directive.ts
+++
b/zeppelin-web-angular/src/app/share/external-links/external-link.directive.ts
@@ -10,7 +10,8 @@
* limitations under the License.
*/
-import { Directive, ElementRef, HostBinding, Input, OnChanges } from
'@angular/core';
+import { Directive, ElementRef, HostBinding, Input, OnChanges, SecurityContext
} from '@angular/core';
+import { DomSanitizer } from '@angular/platform-browser';
@Directive({
// eslint-disable-next-line
@@ -22,10 +23,15 @@ export class ExternalLinkDirective implements OnChanges {
@HostBinding('attr.target') targetAttr: HTMLAnchorElement['target'] | null =
null;
@Input() href?: string;
- constructor(private elementRef: ElementRef) {}
+ constructor(
+ private elementRef: ElementRef,
+ private sanitizer: DomSanitizer
+ ) {}
ngOnChanges() {
- this.elementRef.nativeElement.href = this.href;
+ // This directive captures the `href` input, so Angular's built-in URL
sanitization
+ // for `[href]` bindings is skipped. Sanitize explicitly before writing to
the DOM.
+ this.elementRef.nativeElement.href =
this.sanitizer.sanitize(SecurityContext.URL, this.href ?? null) ?? '';
if (this.isLinkExternal()) {
// https://developers.google.com/web/tools/lighthouse/audits/noopener
