audit-fix sprint: all 13 findings (CRIT/HIGH/MED/LOW)

CRIT-1: ExtractionError::Deobfuscation is now switchable.
        Deobfuscator gains has_sig()/has_nsig() — deobfuscate_sig/_nsig
        short-circuit with a recognisable error class so cipher streams
        on the wrong client fall through to the next client in the chain
        instead of killing the whole call.

CRIT-2: Soft-failed DeobfData now caches with a 1-hour retry instead of
        living for 24h. Re-extraction kicks in automatically once YT
        rotates back to a player.js shape we recognise — no more
        wall-clock-day-of-poisoned-cache.

HIGH-1: Reporter now emits a Level::WRN `extract_deobf_soft_fail` report
        on partial extraction. straw / torttube get an artefact when
        sig/nsig regex starts missing.

HIGH-2: player_client_order branches on opts.auth. With botguard
        + authed-cookie users, Desktop is now position 2 (where their
        cookie maps to an OAuth session) instead of position 4.

HIGH-3: Android dropped from the default order. needs_po_token doesn't
        flag Android, so requests were firing unsigned and tripping
        YT's bot-check rejection — which is also not switchable.
        Re-add when a real po_token strategy lands.

MED-1: Comment in needs_deobf softened — the iOS/Android-no-deobf
        property is a current YT behaviour, not a permanent protocol.

MED-2: Cargo.toml workspace pin bumped 0.11.4 → 0.11.5 so it matches
        the package version (avoids future 0.12.x bump surprises).

MED-3: Smoke test fixture uses an isolated per-process scratch dir
        instead of the repo root, avoiding cache-race with
        tests/youtube.rs (which uses CARGO_MANIFEST_DIR and could
        wipe OAuth tokens).

LOW-1: Misleading "dead-code fallback" comment in extract_fns replaced
        with the actual behaviour description.

LOW-2: get_deobf_data uses read-then-write — concurrent player calls
        on warm cache no longer serialise on the write lock.

LOW-3: Smoke test catches IpBan via exact UnavailabilityReason match
        instead of substring "Sign in/IpBan/bot" — a real regression
        won't silently pass anymore.

LOW-4: TV smoke test now asserts !audio_streams.is_empty() too,
        matching iOS / default-order tests.

LOW-5: needs_deobf comment notes YT's historical n= experiments on
        Android — sets expectation for future review passes.
This commit is contained in:
Kayos 2026-05-24 12:20:14 -07:00
parent 8d7f6b4455
commit 8126cc0da5
6 changed files with 283 additions and 96 deletions

View file

@ -74,7 +74,7 @@ path_macro = "1.0.0"
tracing-test = "0.2.5"
# Included crates
rustypipe = { path = ".", version = "0.11.4", default-features = false }
rustypipe = { path = ".", version = "0.11.5", default-features = false }
rustypipe-downloader = { path = "./downloader", version = "0.3.1", default-features = false, features = [
"indicatif",
"audiotag",

View file

@ -100,10 +100,16 @@ impl ClientType {
}
fn needs_deobf(self) -> bool {
// Android + iOS InnerTube paths return pre-signed stream URLs (no &s= cipher,
// no &n= throttling param), so they don't need player.js deobfuscation at all.
// Skipping the deobf fetch here keeps the player path alive even when YouTube
// rotates the player.js to a shape our extractor doesn't recognise.
// As of YT iOS client v19.x and Android InnerTube v19.x (Dec 2024+),
// both paths return pre-signed stream URLs (no &s= cipher param, no
// &n= throttling param), so they don't need player.js deobfuscation.
// YT has experimented with serving cipher streams to iOS and n= params
// to Android in past A/B tests — if that returns, this matcher needs
// to be revisited along with the po_token strategy and a switchable
// fallback to a Tv/Desktop client.
//
// Skipping the deobf fetch here keeps the player path alive even when
// YouTube rotates player.js to a shape our extractor doesn't recognise.
!matches!(self, ClientType::Ios | ClientType::Android)
}
@ -1276,55 +1282,87 @@ impl RustyPipe {
/// Get deobfuscation data (either from cache or extracted from YouTube's JavaScript code)
async fn get_deobf_data(&self) -> Result<DeobfData, Error> {
// Cheap read-path first: avoid serialising concurrent player calls behind
// the write lock when the cache is already fresh (the common case after
// the first request). Only escalate to a write lock on cache miss.
// (Sulkta fork audit LOW-2.)
{
let read_guard = self.inner.cache.deobf.read().await;
if let Some(data) = read_guard.get() {
return Ok(data.clone());
}
}
// Write lock here to prevent concurrent tasks from fetching the same data
let mut deobf_data = self.inner.cache.deobf.write().await;
match deobf_data.get() {
Some(deobf_data) => Ok(deobf_data.clone()),
None => {
// Only attempt to fetch deobf data every 24 hours to avoid a flood of error reports
// if the client JS cannot be parsed
if deobf_data.should_retry() {
tracing::debug!("getting deobf data");
// Recheck under the write lock — another writer may have filled the cache
// between the read drop and the write acquire.
if let Some(data) = deobf_data.get() {
return Ok(data.clone());
}
match DeobfData::extract(&self.inner.http, self.inner.reporter.as_deref()).await
{
Ok(new_data) => {
// Write new data to the cache
*deobf_data = CacheEntry::from(new_data.clone());
drop(deobf_data);
self.store_cache().await;
Ok(new_data)
}
Err(e) => {
// Try to fall back to expired cache data if available, otherwise return error
deobf_data.retry_later(24);
let res = match deobf_data.get_expired() {
Some(d) => {
tracing::warn!("could not get new deobf data ({e}), falling back to expired cache");
Ok(d.clone())
}
None => Err(e),
};
drop(deobf_data);
self.store_cache().await;
res
}
// Only attempt to fetch deobf data every 24 hours to avoid a flood of error reports
// if the client JS cannot be parsed
if deobf_data.should_retry() {
tracing::debug!("getting deobf data");
match DeobfData::extract(&self.inner.http, self.inner.reporter.as_deref()).await
{
Ok(new_data) => {
// Sulkta fork (audit CRIT-2): soft-failed DeobfData (missing
// sig_fn or nsig_fn) is cached with a much shorter freshness
// window so the next player call retries extraction soon, in
// case YouTube has rotated player.js back to a shape we
// recognise. Otherwise a single rotation could poison the
// cache for the full 24h freshness window even if YT fixed
// things minutes later.
let mut entry = CacheEntry::from(new_data.clone());
if !new_data.is_complete() {
// Force re-extraction on the next request after ~1h
// (so AddrLane workers and short-lived processes get a
// chance to recover without restarting), AND stamp
// failed_version so a library version bump also triggers
// re-extraction immediately.
entry.retry_later(1);
tracing::warn!(
"deobf data partial (has_sig={}, has_nsig={}); caching with 1h retry",
new_data.has_sig(),
new_data.has_nsig()
);
}
} else {
match deobf_data.get_expired() {
*deobf_data = entry;
drop(deobf_data);
self.store_cache().await;
Ok(new_data)
}
Err(e) => {
// Try to fall back to expired cache data if available, otherwise return error
deobf_data.retry_later(24);
let res = match deobf_data.get_expired() {
Some(d) => {
tracing::warn!(
"could not get new deobf data, falling back to expired cache"
);
tracing::warn!("could not get new deobf data ({e}), falling back to expired cache");
Ok(d.clone())
}
None => Err(Error::Extraction(ExtractionError::Deobfuscation(
"could not get deobf data".into(),
))),
}
None => Err(e),
};
drop(deobf_data);
self.store_cache().await;
res
}
}
} else {
match deobf_data.get_expired() {
Some(d) => {
tracing::warn!(
"could not get new deobf data, falling back to expired cache"
);
Ok(d.clone())
}
None => Err(Error::Extraction(ExtractionError::Deobfuscation(
"could not get deobf data".into(),
))),
}
}
}

View file

@ -245,14 +245,31 @@ impl RustyPipeQuery {
/// The order may change in the future in case YouTube applies changes to their
/// platform that disable a client or make it less reliable.
pub fn player_client_order(&self) -> &'static [ClientType] {
// Default to iOS first — it skips player.js deobfuscation entirely (pre-signed
// stream URLs) AND doesn't require device attestation the way the Android
// client does. Tv is the secondary fallback (it does need a sig_timestamp
// request param, but its responses are typically OK). Android is included
// when botguard/po_token signing is wired because then we can satisfy YT's
// device attestation requirement.
// iOS first — it skips player.js deobfuscation entirely (pre-signed
// stream URLs) AND doesn't require device attestation the way Android
// does. Tv is the secondary fallback (needs sig_timestamp in the
// request payload, but the soft-fail extraction keeps that piece alive
// even when sig_fn/nsig_fn extraction breaks).
//
// Android is intentionally NOT in the order: `needs_po_token` doesn't
// flag Android, so requests would fire unsigned and increasingly trip
// YouTube's "Sign in to confirm you're not a bot" — and that mapping
// becomes Unavailable{Captcha} which is not switchable. Re-add when
// a real po_token strategy for Android lands.
//
// Desktop is only consulted when botguard is wired (po_token signing
// available). For authenticated-via-cookie users on botguard
// sessions we put Desktop second so they don't walk through three
// wrong clients before reaching the one their cookie works on.
if self.client.inner.botguard.is_some() {
&[ClientType::Ios, ClientType::Android, ClientType::Tv, ClientType::Desktop]
if self.opts.auth == Some(true) {
// Authed-cookie users: prefer Desktop second (where the cookie
// actually maps to an OAuth session), Tv third (OAuth token),
// iOS first as a quick anonymous path.
&[ClientType::Ios, ClientType::Desktop, ClientType::Tv]
} else {
&[ClientType::Ios, ClientType::Tv, ClientType::Desktop]
}
} else {
&[ClientType::Ios, ClientType::Tv]
}

View file

@ -15,6 +15,28 @@ use crate::{
pub struct Deobfuscator {
ctx: Context,
has_sig: bool,
has_nsig: bool,
}
impl DeobfData {
/// True when both sig_fn and nsig_fn were extracted from the player.js.
/// Used by the cache layer to stamp partial extractions with a shorter
/// retry window than fully-good ones (see `get_deobf_data`).
pub fn is_complete(&self) -> bool {
!self.sig_fn.is_empty() && !self.nsig_fn.is_empty()
}
/// True when the signature deobfuscation fn was extracted successfully.
pub fn has_sig(&self) -> bool {
!self.sig_fn.is_empty()
}
/// True when the throttling-parameter (nsig) deobfuscation fn was
/// extracted successfully.
pub fn has_nsig(&self) -> bool {
!self.nsig_fn.is_empty()
}
}
#[derive(Debug, Default, Clone, Serialize, Deserialize, PartialEq, Eq)]
@ -28,7 +50,10 @@ pub struct DeobfData {
impl DeobfData {
/// Download and extract the latest deobfuscation data from YouTube
///
/// Creates a report if the data could not be extracted
/// Creates a report if the data could not be extracted, including a
/// `Level::WARN` report on partial (sig_fn / nsig_fn) extraction failure
/// so reporter-based consumers (e.g. `FileReporter`) get an artefact to
/// debug new player.js shapes against.
pub async fn extract(http: &Client, reporter: Option<&dyn Reporter>) -> Result<Self, Error> {
let js_url = get_player_js_url(http).await?;
let player_js = get_response(http, &js_url).await?;
@ -36,26 +61,64 @@ impl DeobfData {
let res = Self::extract_fns(&js_url, &player_js);
if let Err(e) = &res {
if let Some(reporter) = reporter {
let report = Report {
info: RustyPipeInfo::new(None, None),
level: Level::ERR,
operation: "extract_deobf",
error: Some(e.to_string()),
msgs: vec![],
deobf_data: None,
http_request: crate::report::HTTPRequest {
url: &js_url,
method: "GET",
req_header: None,
req_body: None,
status: 200,
resp_body: player_js,
},
};
reporter.report(&report);
match &res {
Err(e) => {
if let Some(reporter) = reporter {
let report = Report {
info: RustyPipeInfo::new(None, None),
level: Level::ERR,
operation: "extract_deobf",
error: Some(e.to_string()),
msgs: vec![],
deobf_data: None,
http_request: crate::report::HTTPRequest {
url: &js_url,
method: "GET",
req_header: None,
req_body: None,
status: 200,
resp_body: player_js,
},
};
reporter.report(&report);
}
}
Ok(data) if !data.is_complete() => {
// Soft-fail observability — without this, a sig/nsig extraction
// regression is invisible to reporter-based consumers and only
// shows up at `RUST_LOG=warn`. straw / torttube depend on the
// reporter for in-app crash dumps.
if let Some(reporter) = reporter {
let mut missing = Vec::with_capacity(2);
if !data.has_sig() {
missing.push("sig_fn");
}
if !data.has_nsig() {
missing.push("nsig_fn");
}
let report = Report {
info: RustyPipeInfo::new(None, None),
level: Level::WRN,
operation: "extract_deobf_soft_fail",
error: Some(format!(
"partial extraction; missing: {}",
missing.join(", ")
)),
msgs: vec![],
deobf_data: Some(data.clone()),
http_request: crate::report::HTTPRequest {
url: &js_url,
method: "GET",
req_header: None,
req_body: None,
status: 200,
resp_body: player_js,
},
};
reporter.report(&report);
}
}
Ok(_) => {}
}
res
}
@ -71,9 +134,10 @@ impl DeobfData {
// (iOS, Android, Tv) get pre-signed URLs and never touch these.
// Tolerate extraction failures here so a single rotated player.js
// shape doesn't bring down the whole player path for those clients.
// The dead-code fallback is preserved: if a stream URL DOES need
// deobfuscation, `Deobfuscator::deobfuscate_sig` will fail with a
// clear "sig fn unavailable" error instead of crashing the player.
// When a stream URL DOES carry &s= / &n=, `Deobfuscator::deobfuscate_sig`
// / `deobfuscate_nsig` short-circuit with a switchable error class
// (see `ExtractionError::switch_client` whitelist) so the client
// fallback loop tries the next client instead of killing the call.
let sig_fn = match get_sig_fn(player_js) {
Ok(f) => f,
Err(e) => {
@ -103,29 +167,58 @@ impl Deobfuscator {
pub fn new(data: &DeobfData) -> Result<Self, DeobfError> {
let rt = Runtime::new()?;
let ctx = Context::full(&rt)?;
let has_sig = data.has_sig();
let has_nsig = data.has_nsig();
ctx.with(|ctx| -> Result<(), rquickjs::Error> {
// Skip JS eval for any deobf fn we couldn't extract. The matching
// `deobfuscate_sig` / `deobfuscate_nsig` calls will then return an
// Err naturally because the global won't be defined — and that
// only matters if a stream actually has obfuscated params, which
// shouldn't happen on the iOS/Android/Tv InnerTube paths.
if !data.sig_fn.is_empty() {
// `deobfuscate_sig` / `deobfuscate_nsig` calls below guard on
// `has_sig` / `has_nsig` and short-circuit with a clean
// `sig fn unavailable` error instead of falling into rquickjs
// and getting an opaque `FromJs { from: "undefined" ... }` —
// and that opaque shape used to land in `ExtractionError::Deobfuscation`
// which the upstream client-fallback loop treats as non-switchable.
if has_sig {
let mut opts = rquickjs::context::EvalOptions::default();
opts.strict = false;
ctx.eval_with_options::<(), _>(data.sig_fn.as_bytes(), opts)?;
}
if !data.nsig_fn.is_empty() {
if has_nsig {
let mut opts = rquickjs::context::EvalOptions::default();
opts.strict = false;
ctx.eval_with_options::<(), _>(data.nsig_fn.as_bytes(), opts)?;
}
Ok(())
})?;
Ok(Self { ctx })
Ok(Self {
ctx,
has_sig,
has_nsig,
})
}
/// True when the underlying DeobfData had a valid sig fn extracted.
pub fn has_sig(&self) -> bool {
self.has_sig
}
/// True when the underlying DeobfData had a valid nsig fn extracted.
pub fn has_nsig(&self) -> bool {
self.has_nsig
}
/// Deobfuscate the `s` parameter from the `signature_cipher` field
pub fn deobfuscate_sig(&self, sig: &str) -> Result<String, DeobfError> {
if !self.has_sig {
// Short-circuit with a recognisable error class. Goes through
// `From<DeobfError> for ExtractionError` → `ExtractionError::Deobfuscation`,
// which is in `switch_client`'s whitelist as of the Sulkta fork — so
// `player_from_clients` will try the next client (typically iOS,
// which doesn't carry signature_cipher streams) rather than killing
// the whole call.
return Err(DeobfError::Other(
"sig fn unavailable (player.js rotation; deobf extraction soft-failed)".into(),
));
}
let res = self
.ctx
.with(|ctx| call_fn(&ctx, DEOBF_SIG_FUNC_NAME, sig))?;
@ -135,6 +228,13 @@ impl Deobfuscator {
/// Deobfuscate the `n` stream URL parameter to circumvent throttling
pub fn deobfuscate_nsig(&self, nsig: &str) -> Result<String, DeobfError> {
if !self.has_nsig {
// Same short-circuit as deobfuscate_sig — switchable error class
// for the client-fallback loop instead of an opaque rquickjs panic.
return Err(DeobfError::Other(
"nsig fn unavailable (player.js rotation; throttle deobf soft-failed)".into(),
));
}
let res = self
.ctx
.with(|ctx| call_fn(&ctx, DEOBF_NSIG_FUNC_NAME, nsig))?;

View file

@ -259,6 +259,13 @@ impl ExtractionError {
..
} | ExtractionError::WrongResult(_)
| ExtractionError::Botguard(_)
// Sulkta fork (CRIT-1): deobf failures are usually transient
// — YT rotated the player.js to a shape our regex doesn't
// recognise, or served a cipher stream to a client that doesn't
// have a working sig fn in cache. Switching to another client
// (iOS first, which doesn't need deobf at all) is the right
// recovery move rather than killing the whole call.
| ExtractionError::Deobfuscation(_)
)
}

View file

@ -5,19 +5,35 @@
//!
//! Run with: `cargo test --test sulkta_smoke -- --nocapture`
use std::path::PathBuf;
use rstest::{fixture, rstest};
use rustypipe::client::{ClientType, RustyPipe};
use rustypipe::error::{Error, ExtractionError, UnavailabilityReason};
/// A stable, long-running, public-domain music video. Used by upstream
/// tests too (`n4tK7LYFxI0` = Spektrem - Shine, NCS).
const TEST_VIDEO_ID: &str = "n4tK7LYFxI0";
/// Build a `RustyPipe` with a per-process scratch storage dir. Avoids the
/// concurrent-write race with `tests/youtube.rs` that shares `rustypipe_cache.json`
/// in the repo root, which was tripping audit MED-3.
#[fixture]
fn rp() -> RustyPipe {
let scratch: PathBuf = std::env::temp_dir().join(format!(
"rustypipe-sulkta-smoke-{}-{}",
std::process::id(),
std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.map(|d| d.as_nanos())
.unwrap_or(0)
));
std::fs::create_dir_all(&scratch)
.unwrap_or_else(|e| panic!("create scratch storage dir {scratch:?}: {e}"));
RustyPipe::builder()
.storage_dir(env!("CARGO_MANIFEST_DIR"))
.storage_dir(&scratch)
.build()
.unwrap()
.unwrap_or_else(|e| panic!("build RustyPipe with scratch={scratch:?}: {e}"))
}
/// Sanity: iOS path returns stream URLs and never touches the deobf code.
@ -47,8 +63,9 @@ async fn ios_player_returns_streams(rp: RustyPipe) {
///
/// YouTube IP-bans some shared egress IPs (datacenters, LAN-routed servers)
/// for the TV client with "Sign in to confirm you're not a bot". That's
/// environmental, not a rustypipe regression, so we tolerate it here as long
/// as the error is recognisable.
/// environmental — match it precisely on the `UnavailabilityReason` enum
/// instead of substring-matching the rendered error so a real regression
/// can't sneak past the catch arm.
#[rstest]
#[tokio::test]
async fn tv_player_returns_streams(rp: RustyPipe) {
@ -63,15 +80,22 @@ async fn tv_player_returns_streams(rp: RustyPipe) {
!pd.video_streams.is_empty() || !pd.video_only_streams.is_empty(),
"TV path returned no video streams"
);
}
Err(e) => {
let msg = format!("{e}");
// Symmetric with iOS / default-order tests so a regression that
// silently drops the audio adaptation set can't pass here.
assert!(
msg.contains("Sign in") || msg.contains("IpBan") || msg.contains("bot"),
"TV path failed for a non-environmental reason: {msg}"
!pd.audio_streams.is_empty(),
"TV path returned no audio streams"
);
eprintln!("TV path skipped: YT IP-banned this egress (expected on shared/datacenter IPs)");
}
Err(Error::Extraction(ExtractionError::Unavailable {
reason: UnavailabilityReason::IpBan,
..
})) => {
eprintln!(
"TV path skipped: YT IpBan on this egress (expected on shared/datacenter IPs)"
);
}
Err(e) => panic!("TV path failed for a non-environmental reason: {e}"),
}
}
@ -113,7 +137,10 @@ async fn default_client_order_returns_streams(rp: RustyPipe) {
.expect("at least one audio stream")
.url
.clone();
eprintln!("probing first audio URL: {}", &stream_url[..stream_url.len().min(180)]);
eprintln!(
"probing first audio URL: {}",
&stream_url[..stream_url.len().min(180)]
);
let client = reqwest::Client::builder()
.user_agent(
"com.google.ios.youtube/19.45.4 (iPhone16,2; U; CPU iOS 18_1 like Mac OS X; en_US)",
@ -128,12 +155,10 @@ async fn default_client_order_returns_streams(rp: RustyPipe) {
.expect("GET request to YT CDN should not error");
let status = resp.status();
let body_len = resp.bytes().await.map(|b| b.len()).unwrap_or(0);
eprintln!("response: {} bytes, status {}", body_len, status);
eprintln!("response: {body_len} bytes, status {status}");
assert!(
status.is_success() || status.is_redirection(),
"audio URL Range-GET returned non-OK status: {} (body={} bytes; URL may need visitor_data or po_token)",
status,
body_len
"audio URL Range-GET returned non-OK status: {status} (body={body_len} bytes; URL may need visitor_data or po_token)"
);
assert!(
body_len > 0,