Skip to content

Commit e37690c

Browse files
BunsDevBunsDev
committed
fix: harden terminal link handling
1 parent 0900ef3 commit e37690c

6 files changed

Lines changed: 99 additions & 12 deletions

File tree

‎app/src/terminal/model/blocks.rs‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2420,21 +2420,31 @@ impl BlockList {
24202420
item: &WithinBlock<T>,
24212421
respect_obfuscated_secrets: RespectObfuscatedSecrets,
24222422
) -> String {
2423+
self.try_string_at_range(item, respect_obfuscated_secrets)
2424+
.unwrap_or_default()
2425+
}
2426+
2427+
pub fn try_string_at_range<T: RangeInModel>(
2428+
&self,
2429+
item: &WithinBlock<T>,
2430+
respect_obfuscated_secrets: RespectObfuscatedSecrets,
2431+
) -> Option<String> {
2432+
let block = self.blocks.get(item.block_index.0)?;
24232433
let block_grid = if item.is_in_command_content() {
2424-
self.blocks[item.block_index.0].prompt_and_command_grid()
2434+
block.prompt_and_command_grid()
24252435
} else {
2426-
self.blocks[item.block_index.0].output_grid()
2436+
block.output_grid()
24272437
};
24282438

24292439
let (start, end) = item.inner.range().into_inner();
2430-
block_grid.grid_handler.bounds_to_string(
2440+
Some(block_grid.grid_handler.bounds_to_string(
24312441
start,
24322442
end,
24332443
false,
24342444
respect_obfuscated_secrets,
24352445
false, /* force_obfuscated_secrets */
24362446
RespectDisplayedOutput::Yes,
2437-
)
2447+
))
24382448
}
24392449

24402450
pub fn fragment_boundary_at_point(

‎app/src/terminal/model/terminal_model.rs‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1822,6 +1822,28 @@ impl TerminalModel {
18221822
.to_owned()
18231823
}
18241824

1825+
pub fn try_link_at_range<T: RangeInModel>(
1826+
&self,
1827+
item: &WithinModel<T>,
1828+
respect_obfuscated_secrets: RespectObfuscatedSecrets,
1829+
) -> Option<String> {
1830+
let text = match item {
1831+
WithinModel::AltScreen(inner) => {
1832+
let (start, end) = inner.range().into_inner();
1833+
self.alt_screen
1834+
.bounds_to_string(start, end, respect_obfuscated_secrets)
1835+
}
1836+
WithinModel::BlockList(inner) => self
1837+
.block_list
1838+
.try_string_at_range(inner, respect_obfuscated_secrets)?,
1839+
};
1840+
1841+
Some(
1842+
text.trim_matches(['\u{200B}', ' ', '\n', '\r', '\t'])
1843+
.to_owned(),
1844+
)
1845+
}
1846+
18251847
/// Return all possible file paths containing the grid point ordered from longest to shortest.
18261848
pub fn possible_file_paths_at_point(
18271849
&self,

‎app/src/terminal/view.rs‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15377,7 +15377,7 @@ impl TerminalView {
1537715377
match highlighted_link {
1537815378
GridHighlightedLink::Url(url) => {
1537915379
let url_content =
15380-
Some(model.link_at_range(url, RespectObfuscatedSecrets::Yes));
15380+
model.try_link_at_range(url, RespectObfuscatedSecrets::Yes);
1538115381
url_content
1538215382
.map(|url_content| {
1538315383
vec![MenuItemFields::new("Copy URL")
@@ -17479,9 +17479,15 @@ impl TerminalView {
1747917479
}
1748017480
}
1748117481
GridHighlightedLink::Url(url) if url.contains(position) => {
17482-
let resolved = {
17482+
let Some(resolved) = ({
1748317483
let model = self.model.lock();
17484-
model.link_at_range(url, RespectObfuscatedSecrets::No)
17484+
model.try_link_at_range(url, RespectObfuscatedSecrets::No)
17485+
}) else {
17486+
if self.highlighted_link.take(&mut self.model.lock()).is_some() {
17487+
ctx.reset_cursor();
17488+
ctx.notify();
17489+
}
17490+
return;
1748517491
};
1748617492
ctx.notify();
1748717493
// Route to the embedded browser pane: if one is open in the

‎app/src/terminal/view/link_detection.rs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -389,8 +389,17 @@ impl super::TerminalView {
389389
}
390390
}
391391
GridHighlightedLink::Url(url) => {
392-
let model = self.model.lock();
393-
ctx.open_url(&model.link_at_range(url, RespectObfuscatedSecrets::No));
392+
let resolved = {
393+
let model = self.model.lock();
394+
model.try_link_at_range(url, RespectObfuscatedSecrets::No)
395+
};
396+
397+
if let Some(resolved) = resolved {
398+
ctx.open_url(&resolved);
399+
} else if self.highlighted_link.take(&mut self.model.lock()).is_some() {
400+
ctx.reset_cursor();
401+
ctx.notify();
402+
}
394403
}
395404
};
396405
}

‎app/src/terminal/view_tests.rs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ use crate::terminal::cli_agent_sessions::{
4949
use crate::terminal::model::ansi::{self, InitShellValue};
5050
use crate::terminal::model::ansi::{BootstrappedValue, PreexecValue};
5151
use crate::terminal::model::blocks::{insert_block, TotalIndex};
52+
use crate::terminal::model::grid::grid_handler::Link;
5253
use crate::terminal::model::grid::Dimensions as _;
5354
use crate::terminal::model::terminal_model::WithinBlock;
5455
use crate::terminal::session_settings::AgentToolbarChipSelection;
@@ -3528,6 +3529,31 @@ fn test_link_at_range_trims_zero_width_spaces() {
35283529
})
35293530
}
35303531

3532+
#[test]
3533+
fn test_try_link_at_range_returns_none_for_stale_block_link() {
3534+
App::test((), |mut app| async move {
3535+
initialize_app_for_terminal_view(&mut app);
3536+
let terminal = add_window_with_terminal(&mut app, None);
3537+
3538+
terminal.read(&app, |view, _ctx| {
3539+
let model = view.model.lock();
3540+
let stale_link = WithinModel::BlockList(WithinBlock::new(
3541+
Link {
3542+
range: Point::new(0, 0)..=Point::new(0, 10),
3543+
is_empty: false,
3544+
},
3545+
BlockIndex(model.block_list().blocks().len() + 100),
3546+
crate::terminal::GridType::Output,
3547+
));
3548+
3549+
assert_eq!(
3550+
model.try_link_at_range(&stale_link, RespectObfuscatedSecrets::No),
3551+
None
3552+
);
3553+
});
3554+
})
3555+
}
3556+
35313557
#[test]
35323558
fn test_scroll_position_doesnt_change_when_block_finished() {
35333559
use futures_lite::StreamExt;

‎crates/integration/src/test/ssh.rs‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,12 @@ macro_rules! generate_can_bootstrap_legacy_ssh_test_for_shell {
166166
// TODO(CORE-2333) PowerShell has no SSH wrapper.
167167
// Requires the GCP IAP SSH integration testing VM. Set
168168
// CAST_CODES_SSH_INTEGRATION_TESTS=1 to opt in.
169-
if std::env::var("CAST_CODES_SSH_INTEGRATION_TESTS").ok().as_deref() != Some("1") {
169+
.set_should_run_test(|| {
170+
if std::env::var("CAST_CODES_SSH_INTEGRATION_TESTS")
171+
.ok()
172+
.as_deref()
173+
!= Some("1")
174+
{
170175
return false;
171176
}
172177
if FeatureFlag::SSHTmuxWrapper.is_enabled() {
@@ -264,7 +269,12 @@ macro_rules! generate_long_running_block_ssh_test_for_shell {
264269
// TODO(CORE-2333) PowerShell has no SSH wrapper.
265270
// Requires the GCP IAP SSH integration testing VM. Set
266271
// CAST_CODES_SSH_INTEGRATION_TESTS=1 to opt in.
267-
if std::env::var("CAST_CODES_SSH_INTEGRATION_TESTS").ok().as_deref() != Some("1") {
272+
.set_should_run_test(|| {
273+
if std::env::var("CAST_CODES_SSH_INTEGRATION_TESTS")
274+
.ok()
275+
.as_deref()
276+
!= Some("1")
277+
{
268278
return false;
269279
}
270280
let (starter, _) = current_shell_starter_and_version();
@@ -317,7 +327,11 @@ pub fn test_ssh_with_shell_override() -> Builder {
317327
// Requires the GCP IAP SSH integration testing VM. Set
318328
// CAST_CODES_SSH_INTEGRATION_TESTS=1 to opt in.
319329
.set_should_run_test(|| {
320-
if std::env::var("CAST_CODES_SSH_INTEGRATION_TESTS").ok().as_deref() != Some("1") {
330+
if std::env::var("CAST_CODES_SSH_INTEGRATION_TESTS")
331+
.ok()
332+
.as_deref()
333+
!= Some("1")
334+
{
321335
return false;
322336
}
323337
let (starter, _) = current_shell_starter_and_version();

0 commit comments

Comments
 (0)