Feature - Implemented Removal of "Update" Function (Discuss!)

Grotfang

Developer
I like the fact that chat now allows you to see when someone asks for an update on chat, but should it be default behaviour for an individual to automatically send someone a log of previous conversation on request?

I know this isn't a huge issue, but I feel there should probably be some small discussion about its existence. Holatuwol's post on the function can be found in this post but to summarise it kmails the last 30 messages of clan chat to a clan member if they send you a PM saying "update". Hola believed them to be timestamped (maybe they were at one point), but they are not any longer. In addition, whether you choose to log chat or not is irrelevant -- mafia retains those 30 anyways. The final point is that you can't see who posted what, just what is said, so the information is of limited use.

My concerns:

ClanManager's list of acceptable people to respond to is loaded once per session. If someone moves to my clan and I send update (since it is invoked so rarely) odds are I will get a transcript of the other clan's messages. The other way this can work (and more worryingly) if someone starts in my clan and invokes ClanManager's list, or I do an update through them, when they move I have access to their new clan's chat. I doubt this is intended behaviour.

This was at least slightly mitigated through my recent patch that at least allowed players to see if someone sent them an "update" command. However, I'm not convinced it is a particularly desirable function. Really I'm looking for feedback from folks -- is it good or bad? I'm aware this isn't a well known functionality (certainly mafia users in my clan weren't aware of it and one of them has commit access), but it does exist and it would be good to know whether you think it should.
 

Fluxxdog

Active member
It seems the major issue to me would be bugginess. I'll admit, this functionality seems to be best for clanbots since that's all they're there for is the clan.
 

Theraze

Active member
How about making a preference for the chat tab where it can be enabled or disabled, disabled by default? I could see this as useful for a clanbot, but not useful as a stealth function for most people that gets returned invisibly.

If disabled, there should be a way to manually send the update to someone... possibly by giving a prompted request in the chatbox if someone requests it? Could have it that if you pm confirmed update <name> or update confirm <name> or something like that, it sends them the update, but only if they requested it, and only once, so it can't be used to spam-hit someone.
 

Grotfang

Developer
The way I see it there are three ways this could be changed:

1) Make it work only for buffbots (I don't like this idea, since chat is not buffing)
2) Add a preference to turn it on (default off), possibly without a UI setting, since it is so niche
3) Print something in the gCLI so that at least people know what is happening when someone sends a PM of "update"

I favour a combination of 2 and 3. Would that be acceptable to you folks, or is yet another preference too much? Another possibility is to limit it to people who opt in to log chat (although they aren't really the same thing).
 
Improve handling of automated clan-log request responses

Hello everyone,

Many of you may not be aware of this but if you run Mafia and a fellow clan member sends you a private chat message saying "update", you will automatically respond with a kmail listing recent events in /clan. While this feature is certainly useful at times, it raises several privacy-related concerns:
- it is more-or-less undocumented yet is enabled by default, for everyone using Mafia to chat (unlike some other remote chat commands which are restricted to active buffbots),
- it is impossible to turn off without modifying Mafia source code,
- IMHO Mafia is to quiet about what it does here. Sure, information about a kmail having been sent appears in the status bar, the gCLI window(s) and the log, plus you see the update request in chat - but that is it. In particular, if one only looks at the private-message chat tab containing the request one may get the impression that nothing special has happened. To put it another way: with enough activity in Mafia to obscure the messages, saving sent kmail switched off and when the user is not aware of what "update" does (which is hardly uncommon, given this feature is virtually undocumented), it is entirely possible for one to have no idea one's Mafia has actually done something here,
- there have been reports of certain incorrect Mafia behaviour related to this feature. I shan't go into details here - the devs should already be aware of them and this problem could be exploited by malicious users.

The attached patch attempts to address these issues the following ways:
- there is now a checkbox in preferences which toggles this feature,
- it is off by default,
- regardless of whether a request is handled or denied, an appropriate notification is displayed, in red, in chat.
It also has Mafia reply with a private message if the request has been denied so that the sender knows the recipient does not handle such requests. What exactly should be sent back is of course up to you.

I have tested this patch in my /clan and it seems to work fine. The only thing which could possibly be improved here is to reorder the message in the PM window - at present it is as follows:
1. "request denied" response, if applicable
2. notification message
3. "update" from the sender

Do let me know if you have any questions or comments!
 

Attachments

  • 0001-Only-respond-to-clan-log-requests-from-clan-members-.patch
    3.9 KB · Views: 33
Last edited:

Winterbay

Active member
Why is there a legitimate privacy concern between parties who have /clan in common?

I think that comes form this statement:
- there have been reports of certain incorrect Mafia behaviour related to this feature. I shan't go into details here - the devs should already be aware of them and this problem could be exploited by malicious users.
 

fronobulax

Developer
Staff member
the devs should already be aware of them and this problem could be exploited by malicious users.

Some devs. Not me. I stand by the question if the only users who could exploit the feature are those in /clan but it is a moot point since other devs seem to be aware of it and have not considered it enough of a problem to correct. I'm going to let them make the call.
 

lostcalpolydude

Developer
Staff member
There are a ton of people that use Bonus Adventures from Hell as their rollover clan, or even for VIP access. If you switch over there after a long conversation in some other /clan, and then someone else in there sends you "update", mafia will check the whitelist for BAFH (since it had no reason to check earlier in the session), see that you're currently in the same clan, and send them all the stuff from a clan they aren't part of. Or I might go visit some clan where I have a visitor whitelist (not that "visitor" means anything technically, but generally it means I don't want them seeing all of what happened in my home /clan) and get an "update" in response.
 

Theraze

Active member
Wouldn't that mean the 'proper' response would be to wipe the clan chat history in terms of update when you change clans? Then this whole security thing becomes moot... because all you can see if the clan chat for the clan you're both currently a part of. Also reduces the possibility of spies (ooh!) doing clan chat dumps on purpose...
 

Grotfang

Developer
Wouldn't that mean the 'proper' response would be to wipe the clan chat history in terms of update when you change clans?

Yes. The op's solution doesn't actually address his concern.

To elaborate:

- it is more-or-less undocumented yet is enabled by default, for everyone using Mafia to chat (unlike some other remote chat commands which are restricted to active buffbots),

This function exists for other people, not the user. If you can present a valid reason why someone should be unable to view a chat channel they already have access to, then this may become a problem. Please note that while other devs may disagree with me, I do not view being able to be mean about people while they aren't there as a valid "problem".

- it is impossible to turn off without modifying Mafia source code,
- IMHO Mafia is to quiet about what it does here. Sure, information about a kmail having been sent appears in the status bar, the gCLI window(s) and the log, plus you see the update request in chat - but that is it. In particular, if one only looks at the private-message chat tab containing the request one may get the impression that nothing special has happened. To put it another way: with enough activity in Mafia to obscure the messages, saving sent kmail switched off and when the user is not aware of what "update" does (which is hardly uncommon, given this feature is virtually undocumented), it is entirely possible for one to have no idea one's Mafia has actually done something here,

See above

- there have been reports of certain incorrect Mafia behaviour related to this feature. I shan't go into details here - the devs should already be aware of them and this problem could be exploited by malicious users.

And this is the actual problem you raise.

Please don't see this as an attack -- there is a valid concern here to be addressed. I just haven't so far heard a reasonable explanation for adding user control over this feature. I'm quite happy for our users to (very rarely) be messengers for others. As you say, it is a little-used feature, but one that is useful where it is used.
 
Wouldn't that mean the 'proper' response would be to wipe the clan chat history in terms of update when you change clans?
That would indeed help, Theraze. Alternatively, this whole feature could possibly be nuked altogether... I have just been made aware it is possible to obtain chat history - from before one has logged in, too - from vanilla KoL simply by calling the right script with the right arguments. Then the matter of other people using you for updates you don't want to provide is gone and the possible information leaks become Someone Else's Problem.

I just haven't so far heard a reasonable explanation for adding user control over this feature. I'm quite happy for our users to (very rarely) be messengers for others. As you say, it is a little-used feature, but one that is useful where it is used.
I personally am fine with this too, as long as there is no risk of information leaks associated with it. It is more of a philosophical matter for me - I am a strong believer in that if a piece of software can provide services to other users, especially remotely, the primary one(s) should both be made aware such behaviour can occur, notified whenever it does and, most importantly, have a chance of opting out (or preferably, having it off by default with the possibility of opting in).

That said, I shouldn't have bothered writing and submitting this patch had I not been told about potential security issues in the first place.

Last but not least, please see my comment about being able to do the same without Mafia in response to Theraze.
 
Last edited:

Theraze

Active member
Apparently this is a vanilla KoL problem, according to Pineapple of Despair. We just make it a bit easier, but if I read him properly, anyone can get the chat buffer from anyone else without even needing to log in. As such, mafia is more secure, even at our lowest state, since we at least make an effort to check if the person requesting is a) logged in, and b) in the same clan.

I'd still be a fan of wiping clan chat log history when the user changes clans, but I think the rest of it is fairly irrelevant since it's a KoL bug.
 

lostcalpolydude

Developer
Staff member
So, should something be done with this? It would be trivial to make it opt-in, but there seemed to be resistance to that before. When switching clans, clearing the list of people in your clan and on the whitelist should be done regardless (it looks like a bug that it wasn't happening before), and clearing the list of messages wouldn't be hard.

Please note that while other devs may disagree with me, I do not view being able to be mean about people while they aren't there as a valid "problem".
It seems there is at least one KoL dev that disagrees with this view.

I'm at least going to add some output so people know what is going on. Unless there's a serious objection to it, I plan to add a non-GUI setting to make the feature opt-in, though I will also provide output that tells people what's going on so they can turn it on if someone sends them "update" and it fails.
 

Veracity

Developer
Staff member
I assume you brought this up again because of the pushy and demanding note in the G-D thread. Personally, that raised my hackles. But, if you don't mind being ordered around, by all means, respond.

When I see Pineapple of Despair say "I have just been made aware it is possible to obtain chat history - from before one has logged in, too - from vanilla KoL simply by calling the right script with the right arguments", it's not obvious to me who needs to fix what. Yes, if KoLmafia has an undocumented feature (which I have never thought of using, BTW), perhaps we should, at least, document it.

But when somebody makes a pushy demand in the G-D thread - not even coming here to discuss it - and says that the motivation is because a person can get "Stuff you may have intentionally waited until that person logged off to say." from the chat log, I dig in my heels and resist. You wait until somebody logs off so you can talk trash about them and they later find out what you said? What's to stop one of the people still in the channel from sending your statements to the absent person? Oh, cry me a river.

In other words, I'd really like to understand why this is a "problem" before we rush to put in this or that change.

I'd also like to hear again why hola added this and what he currently thinks about it.
 

lostcalpolydude

Developer
Staff member
It's more that I kind of agree with the idea that it should not be on by default (even if I'm not planning to say stuff that I mind people on my clan's whitelist seeing). I'm probably going to disable it in my local copy regardless of what happens in the main branch.

(That looks like a serious objection, so I won't be charging in to make changes.)
 
Last edited:
Top