In D25775#21206, @valerio.bozzolan wrote:

More understanding on the root cause is needed. Probably the root cause is "just" that getOldValue() returns an empty string. In that case we should probably at least understand what object is that (sub-class of PhabricatorTransactionRemarkupChange?) and we probably we need something like a generateOldValue() or something similar.

After installing subversion and setting LC_ALL instead of LANG I can finally reproduce on a Fedora 40 system:

Oh true, got it. Have to replace PhabricatorPolicyCapability::POLICY_ADMIN, with 'capability' => PhabricatorPolicies::POLICY_ADMIN, here

In D25850#22726, @valerio.bozzolan wrote:

What happens to already-existing URLs? Maybe nice to mention in the test plan

We can also ship this feature in two phases, so, first, adding the option files.maximum-file-size, and then the second one when it's ready or requested lol

Yeah, I agree, though I would then only work on implementing files.maximum-file-size because we don't really care that much about adding exceptions to the rule (as far as I know lol)

last change promise lol

arc unit

harden

\o/

also tried to fix PhutilRemarkupEngineTestCase

but fails in link-edge-cases.txt now (thus it's likely not complete):

Double slam-accept

Uh, that would be so good. So you can say "When the moon is full".

Sounds reasonable.

Take for example this commit that has a default identity:

"Steal credit" might actually lead to a real issue: If a new user can get themselves identified as an old, trusted, user based on commit history, their changes might not be checked as rigorously by the rest of the team - similar to the XZ Utils backdoor issue, only faster.

Limitation: to steal a commit identity, it must be the default. Sorry I forgot to say.

In T15965#20052, @speck wrote:

What can a malicious user accomplish by claiming unverified email for commits?

I like your option names. I like to specify PHIDs and not numeric IDs so it's more portable against import/exports 👍 Let's add Discussion Needed to attract some +1 or nice suggestions.

What happens to already-existing URLs? Maybe nice to mention in the test plan

Fix my local unit test config

Thanks. I see, from this page is not possible:

What can a malicious user accomplish by claiming unverified email for commits? The idea outlined here sounds right but I’d like to understand what potential harm could be done on its current state, and also whether there’s any legitimate use case for the current behavior.

Double slam-accept

Right... one day I may get used to all those Phorge shortcuts, thanks

Oops, no for real

Uhm, right, heh

...like line 105 :)

Thaaanks - If I'm not wrong we can = idx($card, 'objectPHID');

Rebase

I applied this patch locally on top of git master and output does not complain anymore about 'link-brackets.txt' (thus it's correct) but fails in link-edge-cases.txt now (thus it's likely not complete):

run unit test

Adding @aklapper as subscriber in this security issue since I trust this user (unclear if this should be flagged as security thought, feel free to open)