Skip to content

๐Ÿ›ก๏ธ Sentinel: [HIGH] Fix URL ์ธ์ฝ”๋”ฉ๋œ ๊ฒฝ๋กœ ํƒ์ƒ‰ ์ทจ์•ฝ์  (Path Traversal) - #1080

Closed
seonghobae wants to merge 11 commits into
developfrom
security/fix-url-encoded-path-traversal-609834298297248298
Closed

๐Ÿ›ก๏ธ Sentinel: [HIGH] Fix URL ์ธ์ฝ”๋”ฉ๋œ ๊ฒฝ๋กœ ํƒ์ƒ‰ ์ทจ์•ฝ์  (Path Traversal)#1080
seonghobae wants to merge 11 commits into
developfrom
security/fix-url-encoded-path-traversal-609834298297248298

Conversation

@seonghobae

@seonghobae seonghobae commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

๐Ÿšจ Severity: HIGH

๐Ÿ’ก Vulnerability: RFC 6764 CardDAV TXT path hints could hide traversal, backslash, delimiter, or control-character payloads behind one or more percent-encoding layers.

๐ŸŽฏ Impact: A malicious discovery record could escape the intended CardDAV context path after downstream URL decoding.

๐Ÿ”ง Fix:

  • Iteratively percent-decodes the TXT path with a strict five-round budget.
  • Rejects values that still change beyond that budget.
  • Validates the fully decoded value for an absolute path, dot segments, backslashes, URL schemes, query/fragment delimiters, and control characters.
  • Preserves the original safe encoded path for the outbound URL.

โœ… current HEAD verification:

  • Exact current HEAD: 6f99f49679fb1a72c269f1b6ba1b8cc88ec14b96
  • Focused CardDAV discovery suite: 14 passed
  • Full backend: 1541 passed, 33 skipped
  • Ruff, Python compilation, and diff checks: passed
  • CodeGraph re-synced and the bounded decode path plus regressions were verified.
  • Hosted current-head application CI, CodeQL, Semgrep, Bandit, OSV, dependency-review, trivy-fs, Strix, and all three image checks passed.
  • Open code-scanning alerts on the PR ref: 0
  • Unresolved review threads: 0
  • Queued/running previous-head workflow runs: 0

โ„น๏ธ Governance status: Both OpenCode change requests targeting old head 960d23ad818a692def9dc734c0a3594c98415351 were dismissed as stale. They were caused by the coverage sandbox missing the declared fastapi dependency, not by a current code defect. The PR is now REVIEW_REQUIRED, and the metadata gate waits for two new independent approvals. The central dependency-aware deferral fix is tracked in ContextualWisdomLab/.github#590.


PR created automatically by Jules for task 609834298297248298 started by @seonghobae

@google-labs-jules

Copy link
Copy Markdown
Contributor

๐Ÿ‘‹ 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for e0645881d88ada7367ba49c5e9583fdd70a991c5:

  • Review decision is CHANGES_REQUESTED; address requested changes before merge.

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head 960d23ad818a692def9dc734c0a3594c98415351.

  • Head SHA: 960d23ad818a692def9dc734c0a3594c98415351

  • Workflow run: 29569701829

  • Workflow attempt: 1

Coverage evidence

Coverage Decision

  • Result: FAIL
  • Test evidence: not proven passing
  • Docstring evidence: not proven passing when configured
  • Failure count: 1

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (2 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (2 files)"]
  R1 --> V1["required checks"]
  Evidence --> S2["Backend (2 files)"]
  S2 --> I2["API and service runtime"]
  I2 --> R2["Review risk: Backend (2 files)"]
  R2 --> V2["backend tests"]
Loading

@opencode-agent

opencode-agent Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

OpenCode Review Overview

  • Head SHA: e0645881d88ada7367ba49c5e9583fdd70a991c5
  • Workflow run: 29881463449
  • Workflow attempt: 1
  • Gate result: REQUEST_CHANGES (approval step)

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head e0645881d88ada7367ba49c5e9583fdd70a991c5.

  • Head SHA: e0645881d88ada7367ba49c5e9583fdd70a991c5

  • Workflow run: 29881463449

  • Workflow attempt: 1

Coverage evidence

Coverage Decision

  • Result: FAIL
  • Test evidence: not proven passing
  • Docstring evidence: not proven passing when configured
  • Failure count: 1

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (2 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (2 files)"]
  R1 --> V1["required checks"]
  Evidence --> S2["Backend (2 files)"]
  S2 --> I2["API and service runtime"]
  I2 --> R2["Review risk: Backend (2 files)"]
  R2 --> V2["backend tests"]
Loading

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head 960d23ad818a692def9dc734c0a3594c98415351.

  • Head SHA: 960d23ad818a692def9dc734c0a3594c98415351

  • Workflow run: 29569701829

  • Workflow attempt: 2

Coverage evidence

Coverage Decision

  • Result: FAIL
  • Test evidence: not proven passing
  • Docstring evidence: not proven passing when configured
  • Failure count: 1

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (2 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (2 files)"]
  R1 --> V1["required checks"]
  Evidence --> S2["Backend (2 files)"]
  S2 --> I2["API and service runtime"]
  I2 --> R2["Review risk: Backend (2 files)"]
  R2 --> V2["backend tests"]
Loading

@seonghobae

Copy link
Copy Markdown
Contributor Author

current HEAD ๋ณด์•ˆ ์žฌ๊ฐ์‚ฌ โ€” 960d23ad818a692def9dc734c0a3594c98415351

#1085์™€ ๋ณ€๊ฒฝ ๊ฒฝ๋กœ๋ฅผ ๋Œ€์กฐํ–ˆ์Šต๋‹ˆ๋‹ค. #1080์€ CardDAV RFC 6764 TXT context path, #1085๋Š” ์ด๋ฉ”์ผ archive import ๊ฒฝ๋กœ๋ฅผ ๋‹ค๋ฃจ๋ฏ€๋กœ ์ค‘๋ณต PR์ด ์•„๋‹™๋‹ˆ๋‹ค.

CodeGraph๋กœ _txt_context_path -> _srv_base_url -> discover_carddav -> discover_carddav_base_url ๊ฒฝ๋กœ๋ฅผ ์žฌํ™•์ธํ–ˆ์Šต๋‹ˆ๋‹ค. ์ตœ๋Œ€ 5ํšŒ percent-decode ํ›„ ๋‹ค์Œ ํ•ด์„์ด ๋‚จ๋Š” ๊ฐ’, ./.. segment, ์—ญ์Šฌ๋ž˜์‹œ, query/fragment, ์ œ์–ด๋ฌธ์ž ๋ฐ absolute URL ํ˜•ํƒœ๋ฅผ ๊ฑฐ๋ถ€ํ•˜๋ฉฐ ๊ฒ€์ฆ๋œ ์›๋ž˜ ์ธ์ฝ”๋”ฉ ๊ฒฝ๋กœ๋งŒ TLS SRV URL์— ์ „๋‹ฌํ•ฉ๋‹ˆ๋‹ค.

๊ฒ€์ฆ:

  • focused CardDAV: 14 passed
  • backend ์ „์ฒด: 1541 passed, 33 skipped
  • Ruff: ํ†ต๊ณผ
  • Bandit Medium ์ด์ƒ: 0
  • compileall / diff check: ํ†ต๊ณผ
  • current HEAD code-scanning open alert: 0
  • unresolved review thread: 0

๋‚จ์€ CHANGES_REQUESTED๋Š” branch ์ฝ”๋“œ ์‹คํŒจ๊ฐ€ ์•„๋‹ˆ๋ผ ์ค‘์•™ networkless coverage๊ฐ€ base์— ์„ ์–ธ๋œ fastapi๋ฅผ ์„ค์น˜ํ•˜์ง€ ๋ชปํ•œ collection failure์ž…๋‹ˆ๋‹ค. ๋™์ผ ์›์ธ์„ base manifest์— ๊ฒฐ์†ํ•ด native required checks๋กœ ์•ˆ์ „ํ•˜๊ฒŒ deferํ•˜๋Š” ์ค‘์•™ #590์ด ํ˜„์žฌ ๋ฆฌ๋ทฐ ์ค‘์ด๋ฏ€๋กœ, ์ด HEAD์—๋Š” ๋ถˆํ•„์š”ํ•œ ์ฝ”๋“œ ๋ณ€๊ฒฝ์„ ๋งŒ๋“ค์ง€ ์•Š์•˜์Šต๋‹ˆ๋‹ค.

@seonghobae
seonghobae dismissed stale reviews from opencode-agent[bot] and opencode-agent[bot] July 20, 2026 15:30

Stale review cleanup: this review targets 960d23a, while current HEAD is 6f99f49. The bounded defense is restored exactly; focused 14 and full backend 1541/33 tests pass, and current-head application, CodeQL, SAST, dependency, and filesystem security checks pass. The old finding was the central coverage sandbox missing the declared fastapi dependency, not a current code defect.

@seonghobae

Copy link
Copy Markdown
Contributor Author

current HEAD ๋ณด์•ˆ ๋ณต์› ์ฆ๊ฑฐ

  • ์ •ํ™•ํ•œ HEAD: 6f99f49679fb1a72c269f1b6ba1b8cc88ec14b96
  • CardDAV TXT path๋ฅผ ์ตœ๋Œ€ 5ํšŒ ๋””์ฝ”๋”ฉํ•˜๊ณ , ์˜ˆ์‚ฐ ์ดํ›„์—๋„ ๊ฐ’์ด ๋ณ€ํ•˜๋ฉด fail-closedํ•ฉ๋‹ˆ๋‹ค.
  • ์™„์ „ํžˆ ๋””์ฝ”๋”ฉ๋œ ๊ฐ’์—์„œ traversal ์  ์„ธ๊ทธ๋จผํŠธ, ์—ญ์Šฌ๋ž˜์‹œ, URL scheme, query/fragment ๊ตฌ๋ถ„์ž, ์ œ์–ด๋ฌธ์ž๋ฅผ ๊ฑฐ๋ถ€ํ•˜๋ฉฐ ์•ˆ์ „ํ•œ ์›๋ž˜ ์ธ์ฝ”๋”ฉ ๊ฒฝ๋กœ๋งŒ ๋ฐ˜ํ™˜ํ•ฉ๋‹ˆ๋‹ค.
  • ์ง‘์ค‘ ํ…Œ์ŠคํŠธ 14 passed; ์ „์ฒด ๋ฐฑ์—”๋“œ 1541 passed, 33 skipped; Ruffยท์ปดํŒŒ์ผยทdiff ๊ฒ€์‚ฌ ํ†ต๊ณผ
  • CodeGraph ์‚ฌํ›„ ๋™๊ธฐํ™”๋กœ _txt_context_path์™€ ์ธ์ฝ”๋”ฉ ๊ณต๊ฒฉ ํšŒ๊ท€ ๊ฒฝ๋กœ๋ฅผ ํ™•์ธํ–ˆ์Šต๋‹ˆ๋‹ค.
  • current-head ์• ํ”Œ๋ฆฌ์ผ€์ด์…˜ CI, CodeQL, Semgrep, Bandit, OSV, dependency-review, trivy-fs, Strix, ์„ธ ์ด๋ฏธ์ง€ ๊ฒ€์ฆ: ๋ชจ๋‘ ํ†ต๊ณผ
  • PR ref์˜ ์—ด๋ฆฐ ์ฝ”๋“œ์Šค์บ๋‹ ๊ฒฝ๋ณด: 0
  • ๋ฏธํ•ด๊ฒฐ ๋ฆฌ๋ทฐ ์Šค๋ ˆ๋“œ: 0
  • queued/running ์ด์ „ HEAD ์›Œํฌํ”Œ๋กœ: 0
  • ์ด์ „ 960d23adโ€ฆ ๋Œ€์ƒ OpenCode ๋ณ€๊ฒฝ ์š”์ฒญ 2๊ฐœ๋Š” stale๋กœ dismiss๋๊ณ  ํ˜„์žฌ REVIEW_REQUIRED์ž…๋‹ˆ๋‹ค. ๋ฉ”ํƒ€๋ฐ์ดํ„ฐ ๊ฒŒ์ดํŠธ๋Š” ์ƒˆ ๋…๋ฆฝ ์Šน์ธ 2๊ฐœ๋ฅผ ๊ธฐ๋‹ค๋ฆฝ๋‹ˆ๋‹ค.

@seonghobae

seonghobae commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Security-critical bounded CardDAV decoding and its regression coverage have been restored on current HEAD 7f8f602 after the latest automated replay removed them. @jules stop โ€” please do not add further commits to this PR while current-head CI and review are being revalidated.

@seonghobae

Copy link
Copy Markdown
Contributor Author

The Sentinel replay on f322405 removed the bounded recursive decoder and 29 encoded-input regressions after they had already been restored. Current HEAD d4c2749 restores the verified tree; 14 focused tests and the full backend suite (1542 passed, 32 skipped) pass. @sentinel stop @jules stop โ€” do not add further commits while current-head CI and review are revalidated.

@google-labs-jules

Copy link
Copy Markdown
Contributor

Security-critical bounded CardDAV decoding and its regression coverage have been restored on current HEAD after the latest automated replay removed them. @jules stop โ€” please do not add further commits to this PR while current-head CI and review are being revalidated.

Understood. Acknowledging that this work is now obsolete and stopping work on this task.

@google-labs-jules

Copy link
Copy Markdown
Contributor

The Sentinel replay on f322405 removed the bounded recursive decoder and 29 encoded-input regressions after they had already been restored. Current HEAD d4c2749 restores the verified tree; 14 focused tests and the full backend suite (1542 passed, 32 skipped) pass. @sentinel stop @jules stop โ€” do not add further commits while current-head CI and review are revalidated.

Understood. Acknowledging that this work is now obsolete and stopping work on this task.

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head f4c19ee1090c71a17b41ef195e9da4114a3c256e.

  • Head SHA: f4c19ee1090c71a17b41ef195e9da4114a3c256e

  • Workflow run: 29799910969

  • Workflow attempt: 1

Coverage evidence

Coverage Decision

  • Result: FAIL
  • Test evidence: not proven passing
  • Docstring evidence: not proven passing when configured
  • Failure count: 1

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (2 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (2 files)"]
  R1 --> V1["required checks"]
  Evidence --> S2["Backend (2 files)"]
  S2 --> I2["API and service runtime"]
  I2 --> R2["Review risk: Backend (2 files)"]
  R2 --> V2["backend tests"]
Loading

@seonghobae
seonghobae force-pushed the security/fix-url-encoded-path-traversal-609834298297248298 branch from f4c19ee to e064588 Compare July 21, 2026 23:40

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head e0645881d88ada7367ba49c5e9583fdd70a991c5.

  • Head SHA: e0645881d88ada7367ba49c5e9583fdd70a991c5

  • Workflow run: 29881463449

  • Workflow attempt: 1

Coverage evidence

Coverage Decision

  • Result: FAIL
  • Test evidence: not proven passing
  • Docstring evidence: not proven passing when configured
  • Failure count: 1

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (2 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (2 files)"]
  R1 --> V1["required checks"]
  Evidence --> S2["Backend (2 files)"]
  S2 --> I2["API and service runtime"]
  I2 --> R2["Review risk: Backend (2 files)"]
  R2 --> V2["backend tests"]
Loading

@seonghobae

Copy link
Copy Markdown
Contributor Author

Superseded by clean reimplementation of CardDAV encoded path validation on current develop (same tests, no CHANGELOG conflict noise).

@seonghobae seonghobae closed this Jul 31, 2026
@google-labs-jules

Copy link
Copy Markdown
Contributor

Superseded by clean reimplementation of CardDAV encoded path validation on current develop (same tests, no CHANGELOG conflict noise).

Understood. Acknowledging that this work is now obsolete and stopping work on this task.

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.

1 participant