Skip to content

fix: reject links with invalid or empty source/target/predicate URIs #676

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lucksus merged 10 commits into from
Feb 18, 2026
Merged

fix: reject links with invalid or empty source/target/predicate URIs #676

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c540296
fix: reject links with invalid or empty source/target/predicate URIs
lucksus Feb 17, 2026
fa71cdc
adjust unit tests to only produce valid link URIs
lucksus Feb 17, 2026
3b18dfa
expect() -> map_err()
lucksus Feb 18, 2026
ba48ac3
Integration test links -> valid URIs
lucksus Feb 18, 2026
836fecb
Merge branch 'dev' into fix/reject-invalid-links
lucksus Feb 18, 2026
d0b3c24
neighbourhood links -> valid URIs
lucksus Feb 18, 2026
66c112c
Wrap string in literal to prevent bare strings
lucksus Feb 18, 2026
ebad0f1
Fix URI regexp
lucksus Feb 18, 2026
267bc33
prolog-and-literal links -> valid URIs
lucksus Feb 18, 2026
733b6bb
Merge remote-tracking branch 'origin/fix/reject-invalid-links' into f…
lucksus Feb 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
177 changes: 112 additions & 65 deletions rust-executor/src/perspectives/perspective_instance.rs
View file
Open in desktop

Large diffs are not rendered by default.

36 changes: 36 additions & 0 deletions rust-executor/src/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,42 @@ impl Link {
target: self.target.clone(),
}
}

/// Validates that source and target are non-empty URIs (RFC 3986 scheme present),
/// and that predicate, if present, is also a valid URI.
///
/// A valid URI must begin with a scheme: `[a-zA-Z][a-zA-Z0-9+\-._]*:`
/// (underscore is included as a pragmatic extension for schemes like `smart_literal://`)
/// Examples: `did:key:alice`, `expression://Qm...`, `smart_literal://content`
pub fn validate(&self) -> Result<(), AnyError> {
use std::sync::OnceLock;
static URI_SCHEME_RE: OnceLock<Regex> = OnceLock::new();
let re = URI_SCHEME_RE.get_or_init(|| Regex::new(r"^[a-zA-Z][a-zA-Z0-9+\-._]*:").unwrap());

let check = |field: &str, value: &str| -> Result<(), AnyError> {
if value.is_empty() {
return Err(anyhow!("Link {} must not be empty", field));
}
if !re.is_match(value) {
return Err(anyhow!(
"Link {} is not a valid URI (must start with a scheme like 'did:', 'expression://', etc.): '{}'",
field, value
));
}
Ok(())
};

check("source", &self.source)?;
check("target", &self.target)?;

if let Some(predicate) = &self.predicate {
if !predicate.is_empty() {
check("predicate", predicate)?;
}
Comment on lines +111 to +141
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Feb 17, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Reject empty predicate explicitly if it should be invalid.

predicate == "" currently bypasses validation, so empty predicates slip through despite the “reject empty URIs” objective. If empty predicates should be rejected, add an explicit error before calling check().

🛠️ Suggested fix 
-        if let Some(predicate) = &self.predicate {
-            if !predicate.is_empty() {
-                check("predicate", predicate)?;
-            }
-        }
+        if let Some(predicate) = &self.predicate {
+            if predicate.is_empty() {
+                return Err(anyhow!("Link predicate must not be empty"));
+            }
+            check("predicate", predicate)?;
+        }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@rust-executor/src/types.rs` around lines 111 - 140, In validate(), the
current predicate branch allows an empty string to bypass URI validation; update
the predicate handling in the validate() method so that when self.predicate is
Some(predicate) you first return an error if predicate.is_empty() (matching the
behavior used for source/target) and otherwise call the existing check closure;
reference the validate function, the local check closure, and the predicate
variable to locate where to add the explicit empty-string rejection.

}

Ok(())
}
}

#[derive(GraphQLObject, Debug, Deserialize, Serialize, Clone, PartialEq)]
Expand Down
2 changes: 1 addition & 1 deletion tests/js/tests/agent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export default function agentTests(testContext: TestContext) {
let link = new LinkExpression();
link.author = "did:test";
link.timestamp = new Date().toISOString();
link.data = new Link({source: "src", target: "target", predicate: "pred"});
link.data = new Link({source: "ad4m://src", target: "test://target", predicate: "ad4m://pred"});
link.proof = new ExpressionProof("sig", "key")
const updatePerspective = await ad4mClient.agent.updatePublicPerspective(new Perspective([link]))
expect(currentAgent.perspective).not.to.be.undefined
Expand Down
2 changes: 1 addition & 1 deletion tests/js/tests/integration.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ describe("Integration tests", function () {
let link = new LinkExpression();
link.author = "did:test";
link.timestamp = new Date().toISOString();
link.data = new Link({source: "src", target: "target", predicate: "pred"});
link.data = new Link({source: "ad4m://src", target: "test://target", predicate: "ad4m://pred"});
link.proof = new ExpressionProof("sig", "key")

await testContext.bob.agent.updatePublicPerspective(new Perspective([link]))
Expand Down
4 changes: 2 additions & 2 deletions tests/js/tests/multi-user-simple.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ describe("Multi-User Simple integration tests", () => {
const p1 = await client1.perspective.add("User 1 Test Perspective");
// @ts-ignore - Suppress Apollo type mismatch
const link1 = await client1.perspective.addLink(p1.uuid, {
source: "root",
source: "ad4m://root",
target: "test://target1",
predicate: "test://predicate"
});
Expand All @@ -494,7 +494,7 @@ describe("Multi-User Simple integration tests", () => {
const p2 = await client2.perspective.add("User 2 Test Perspective");
// @ts-ignore - Suppress Apollo type mismatch
const link2 = await client2.perspective.addLink(p2.uuid, {
source: "root",
source: "ad4m://root",
target: "test://target2",
predicate: "test://predicate"
});
Expand Down
48 changes: 24 additions & 24 deletions tests/js/tests/neighbourhood.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export default function neighbourhoodTests(testContext: TestContext) {
let link = new LinkExpression()
link.author = "did:test";
link.timestamp = new Date().toISOString();
link.data = new Link({source: "src", target: "target", predicate: "pred"});
link.data = new Link({source: "ad4m://src", target: "test://target", predicate: "ad4m://pred"});
link.proof = new ExpressionProof("sig", "key");
const publishPerspective = await ad4mClient.neighbourhood.publishFromPerspective(create.uuid, socialContext.address,
new Perspective(
Expand Down Expand Up @@ -93,17 +93,17 @@ export default function neighbourhoodTests(testContext: TestContext) {

await sleep(1000)

await alice.perspective.addLink(aliceP1.uuid, {source: 'root', target: 'test://test'})
await alice.perspective.addLink(aliceP1.uuid, {source: 'ad4m://root', target: 'test://test'})

await sleep(1000)

let bobLinks = await bob.perspective.queryLinks(bobP1!.uuid, new LinkQuery({source: 'root'}))
let bobLinks = await bob.perspective.queryLinks(bobP1!.uuid, new LinkQuery({source: 'ad4m://root'}))
let tries = 1

while(bobLinks.length < 1 && tries < 60) {
console.log("Bob retrying getting links...");
await sleep(1000)
bobLinks = await bob.perspective.queryLinks(bobP1!.uuid, new LinkQuery({source: 'root'}))
bobLinks = await bob.perspective.queryLinks(bobP1!.uuid, new LinkQuery({source: 'ad4m://root'}))
tries++
}

Expand All @@ -127,17 +127,17 @@ export default function neighbourhoodTests(testContext: TestContext) {

await sleep(1000)

await alice.perspective.addLink(aliceP1.uuid, {source: 'root', target: 'test://test'}, 'local')
await alice.perspective.addLink(aliceP1.uuid, {source: 'ad4m://root', target: 'test://test'}, 'local')

await sleep(1000)

let bobLinks = await bob.perspective.queryLinks(bobP1!.uuid, new LinkQuery({source: 'root'}))
let bobLinks = await bob.perspective.queryLinks(bobP1!.uuid, new LinkQuery({source: 'ad4m://root'}))
let tries = 1

while(bobLinks.length < 1 && tries < 5) {
console.log("Bob retrying getting NOT received links...");
await sleep(1000)
bobLinks = await bob.perspective.queryLinks(bobP1!.uuid, new LinkQuery({source: 'root'}))
bobLinks = await bob.perspective.queryLinks(bobP1!.uuid, new LinkQuery({source: 'ad4m://root'}))
tries++
}

Expand All @@ -162,22 +162,22 @@ export default function neighbourhoodTests(testContext: TestContext) {
//const linkPromises = []
for(let i = 0; i < 1500; i++) {
console.log("Alice adding link ", i)
const link = await alice.perspective.addLink(aliceP1.uuid, {source: 'root', target: `test://test/${i}`})
const link = await alice.perspective.addLink(aliceP1.uuid, {source: 'ad4m://root', target: `test://test/${i}`})
console.log("Link expression:", link)
}
//await Promise.all(linkPromises)

console.log("wait 15s for initial sync")
await sleep(15000)

let bobLinks = await bob.perspective.queryLinks(bobP1!.uuid, new LinkQuery({source: 'root'}))
let bobLinks = await bob.perspective.queryLinks(bobP1!.uuid, new LinkQuery({source: 'ad4m://root'}))
let tries = 1
const maxTries = 180 // 3 minutes with 1 second sleep (increased for fallback sync)

while(bobLinks.length < 1500 && tries < maxTries) {
console.log(`Bob retrying getting links... Got ${bobLinks.length}/1500`);
await sleep(1000)
bobLinks = await bob.perspective.queryLinks(bobP1!.uuid, new LinkQuery({source: 'root'}))
bobLinks = await bob.perspective.queryLinks(bobP1!.uuid, new LinkQuery({source: 'ad4m://root'}))
tries++
}

Expand All @@ -195,19 +195,19 @@ export default function neighbourhoodTests(testContext: TestContext) {

// Alice creates some links
console.log("Alice creating links...")
await testContext.alice.perspective.addLink(aliceP1.uuid, {source: 'alice', target: 'test://alice/1'})
await testContext.alice.perspective.addLink(aliceP1.uuid, {source: 'alice', target: 'test://alice/2'})
await testContext.alice.perspective.addLink(aliceP1.uuid, {source: 'alice', target: 'test://alice/3'})
await testContext.alice.perspective.addLink(aliceP1.uuid, {source: 'ad4m://alice', target: 'test://alice/1'})
await testContext.alice.perspective.addLink(aliceP1.uuid, {source: 'ad4m://alice', target: 'test://alice/2'})
await testContext.alice.perspective.addLink(aliceP1.uuid, {source: 'ad4m://alice', target: 'test://alice/3'})

// Wait for sync with retry loop
bobLinks = await testContext.bob.perspective.queryLinks(bobP1.uuid, new LinkQuery({source: 'alice'}))
bobLinks = await testContext.bob.perspective.queryLinks(bobP1.uuid, new LinkQuery({source: 'ad4m://alice'}))
let bobTries = 1
const maxTriesBob = 20 // 20 tries with 2 second sleep = 40 seconds max

while(bobLinks.length < 3 && bobTries < maxTriesBob) {
console.log(`Bob retrying getting Alice's links... Got ${bobLinks.length}/3`);
await sleep(2000)
bobLinks = await testContext.bob.perspective.queryLinks(bobP1.uuid, new LinkQuery({source: 'alice'}))
bobLinks = await testContext.bob.perspective.queryLinks(bobP1.uuid, new LinkQuery({source: 'ad4m://alice'}))
bobTries++
}

Expand All @@ -219,19 +219,19 @@ export default function neighbourhoodTests(testContext: TestContext) {

// Bob creates some links
console.log("Bob creating links...")
await testContext.bob.perspective.addLink(bobP1.uuid, {source: 'bob', target: 'test://bob/1'})
await testContext.bob.perspective.addLink(bobP1.uuid, {source: 'bob', target: 'test://bob/2'})
await testContext.bob.perspective.addLink(bobP1.uuid, {source: 'bob', target: 'test://bob/3'})
await testContext.bob.perspective.addLink(bobP1.uuid, {source: 'ad4m://bob', target: 'test://bob/1'})
await testContext.bob.perspective.addLink(bobP1.uuid, {source: 'ad4m://bob', target: 'test://bob/2'})
await testContext.bob.perspective.addLink(bobP1.uuid, {source: 'ad4m://bob', target: 'test://bob/3'})

// Wait for sync with retry loop
let aliceLinks = await testContext.alice.perspective.queryLinks(aliceP1.uuid, new LinkQuery({source: 'bob'}))
let aliceLinks = await testContext.alice.perspective.queryLinks(aliceP1.uuid, new LinkQuery({source: 'ad4m://bob'}))
tries = 1
const maxTriesAlice = 20 // 2 minutes with 1 second sleep

while(aliceLinks.length < 3 && tries < maxTriesAlice) {
console.log(`Alice retrying getting links... Got ${aliceLinks.length}/3`);
await sleep(2000)
aliceLinks = await testContext.alice.perspective.queryLinks(aliceP1.uuid, new LinkQuery({source: 'bob'}))
aliceLinks = await testContext.alice.perspective.queryLinks(aliceP1.uuid, new LinkQuery({source: 'ad4m://bob'}))
tries++
}

Expand Down Expand Up @@ -275,7 +275,7 @@ export default function neighbourhoodTests(testContext: TestContext) {

// aliceP1.addSyncStateChangeListener(aliceSyncChangeHandler);

// await testContext.alice.perspective.addLink(aliceP1.uuid, {source: 'root', target: 'test://test'})
// await testContext.alice.perspective.addLink(aliceP1.uuid, {source: 'ad4m://root', target: 'test://test'})

// let bobSyncChangeCalls = 0;
// let bobSyncChangeData = null;
Expand All @@ -295,12 +295,12 @@ export default function neighbourhoodTests(testContext: TestContext) {

// //These next assertions are flaky since they depend on holochain not syncing right away, which most of the time is the case

// let bobLinks = await testContext.bob.perspective.queryLinks(bobP1!.uuid, new LinkQuery({source: 'root'}))
// let bobLinks = await testContext.bob.perspective.queryLinks(bobP1!.uuid, new LinkQuery({source: 'ad4m://root'}))
// let tries = 1

// while(bobLinks.length < 1 && tries < 300) {
// await sleep(1000)
// bobLinks = await testContext.bob.perspective.queryLinks(bobP1!.uuid, new LinkQuery({source: 'root'}))
// bobLinks = await testContext.bob.perspective.queryLinks(bobP1!.uuid, new LinkQuery({source: 'ad4m://root'}))
// tries++
// }

Expand Down Expand Up @@ -368,7 +368,7 @@ export default function neighbourhoodTests(testContext: TestContext) {
let link = new LinkExpression()
link.author = "did:test";
link.timestamp = new Date().toISOString();
link.data = new Link({source: "src", target: "target", predicate: "pred"});
link.data = new Link({source: "ad4m://src", target: "test://target", predicate: "ad4m://pred"});
link.proof = new ExpressionProof("sig", "key");
link.proof.invalid = true;
link.proof.valid = false;
Expand Down
18 changes: 9 additions & 9 deletions tests/js/tests/perspective.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,13 @@ export default function perspectiveTests(testContext: TestContext) {
expect(perspective.name).to.equal('test-duplicate-link-removal');

// create link
const link = { source: 'root', predicate: 'p', target: 'abc' };
const link = { source: 'ad4m://root', predicate: 'ad4m://p', target: 'test://abc' };
const addLink = await perspective.add(link);
expect(addLink.data.target).to.equal("abc");
expect(addLink.data.target).to.equal("test://abc");

// get link expression
const linkExpression = (await perspective.get(new LinkQuery(link)))[0];
expect(linkExpression.data.target).to.equal("abc");
expect(linkExpression.data.target).to.equal("test://abc");

// attempt to remove link twice (currently errors and prevents further execution of code)
await perspective.removeLinks([linkExpression, linkExpression])
Expand Down Expand Up @@ -291,12 +291,12 @@ export default function perspectiveTests(testContext: TestContext) {
const linkUpdated = sinon.fake()
await ad4mClient.perspective.addPerspectiveLinkUpdatedListener(p1.uuid, [linkUpdated])

const linkExpression = await ad4mClient.perspective.addLink(p1.uuid , {source: 'root', target: 'lang://123'})
const linkExpression = await ad4mClient.perspective.addLink(p1.uuid , {source: 'ad4m://root', target: 'lang://123'})
await sleep(1000)
expect(linkAdded.called).to.be.true;
expect(linkAdded.getCall(0).args[0]).to.eql(linkExpression)

const updatedLinkExpression = await ad4mClient.perspective.updateLink(p1.uuid , linkExpression, {source: 'root', target: 'lang://456'})
const updatedLinkExpression = await ad4mClient.perspective.updateLink(p1.uuid , linkExpression, {source: 'ad4m://root', target: 'lang://456'})
await sleep(1000)
expect(linkUpdated.called).to.be.true;
expect(linkUpdated.getCall(0).args[0].newLink).to.eql(updatedLinkExpression)
Expand Down Expand Up @@ -818,20 +818,20 @@ export default function perspectiveTests(testContext: TestContext) {
const link1 = new Link({
source: 'test://source',
predicate: 'test://predicate',
target: 'target1'
target: 'test://target1'
})

await proxy.setSingleTarget(link1)
const result1 = (await proxy.get(all))[0].data
expect(result1.source).to.equal(link1.source)
expect(result1.predicate).to.equal(link1.predicate)
expect(result1.target).to.equal(link1.target)
expect(await proxy.getSingleTarget(new LinkQuery(link1))).to.equal('target1')
expect(await proxy.getSingleTarget(new LinkQuery(link1))).to.equal('test://target1')

const link2 = new Link({
source: 'test://source',
predicate: 'test://predicate',
target: 'target2'
target: 'test://target2'
})

await proxy.setSingleTarget(link2)
Expand All @@ -840,7 +840,7 @@ export default function perspectiveTests(testContext: TestContext) {
expect(result2.source).to.equal(link2.source)
expect(result2.predicate).to.equal(link2.predicate)
expect(result2.target).to.equal(link2.target)
expect(await proxy.getSingleTarget(new LinkQuery(link1))).to.equal('target2')
expect(await proxy.getSingleTarget(new LinkQuery(link1))).to.equal('test://target2')
})

// SdnaOnly doesn't load links into prolog engine
Expand Down
Loading